Audit __private_extern__ handling.

- Exposed quite a few Sema issues and a CodeGen crash.

 - See FIXMEs in test case, and in SemaDecl.cpp (PR3983).

I'm skeptical that __private_extern__ should actually be a storage
class value. I think that __private_extern__ basically amounts to
  extern A __attribute__((visibility("hidden")))
and would be better off handled (a) as that, or (b) with an extra bit
in the VarDecl.

llvm-svn: 69020

clang/lib/AST/StmtDumper.cpp

@@ -217,14 +217,9 @@ void StmtDumper::DumpDeclarator(Decl *D) {
     fprintf(F, "\"");
     // Emit storage class for vardecls.
     if (VarDecl *V = dyn_cast<VarDecl>(VD)) {
-      switch (V->getStorageClass()) {
-      default: assert(0 && "Unknown storage class!");
-      case VarDecl::None:     break;
-      case VarDecl::Extern:   fprintf(F, "extern "); break;
-      case VarDecl::Static:   fprintf(F, "static "); break; 
-      case VarDecl::Auto:     fprintf(F, "auto "); break;
-      case VarDecl::Register: fprintf(F, "register "); break;
-      }
+      if (V->getStorageClass() != VarDecl::None)
+        fprintf(F, "%s ", 
+                VarDecl::getStorageClassSpecifierString(V->getStorageClass()));
     }
     
     std::string Name = VD->getNameAsString();

clang/lib/AST/StmtPrinter.cpp

@@ -117,15 +117,9 @@ void StmtPrinter::PrintRawDecl(Decl *D) {
   } else if (ValueDecl *VD = dyn_cast<ValueDecl>(D)) {
     // Emit storage class for vardecls.
     if (VarDecl *V = dyn_cast<VarDecl>(VD)) {
-      switch (V->getStorageClass()) {
-        default: assert(0 && "Unknown storage class!");
-        case VarDecl::None:          break;
-        case VarDecl::Extern:        OS << "extern "; break;
-        case VarDecl::Static:        OS << "static "; break; 
-        case VarDecl::Auto:          OS << "auto "; break;
-        case VarDecl::Register:      OS << "register "; break;
-        case VarDecl::PrivateExtern: OS << "__private_extern "; break; 
-      }
+      if (V->getStorageClass() != VarDecl::None)
+        OS << VarDecl::getStorageClassSpecifierString(V->getStorageClass()) 
+           << ' ';
     }
     
     std::string Name = VD->getNameAsString();

clang/lib/CodeGen/CGBlocks.cpp

@@ -608,8 +608,7 @@ CodeGenFunction::GenerateBlockFunction(const BlockExpr *BExpr,
        ++i) {
     const VarDecl *VD = dyn_cast<VarDecl>(i->first);
     
-    if (VD->getStorageClass() == VarDecl::Static
-        || VD->getStorageClass() == VarDecl::Extern)
+    if (VD->getStorageClass() == VarDecl::Static || VD->hasExternalStorage())
       LocalDeclMap[VD] = i->second;
   }
 

clang/lib/CodeGen/CGDecl.cpp

@@ -68,18 +68,19 @@ void CodeGenFunction::EmitBlockVarDecl(const VarDecl &D) {
     CGM.ErrorUnsupported(&D, "thread local ('__thread') variable", true);
   
   switch (D.getStorageClass()) {
+  case VarDecl::None:
+  case VarDecl::Auto:
+  case VarDecl::Register:
+    return EmitLocalBlockVarDecl(D);
   case VarDecl::Static:
     return EmitStaticBlockVarDecl(D);
   case VarDecl::Extern:
+  case VarDecl::PrivateExtern:
     // Don't emit it now, allow it to be emitted lazily on its first use.
     return;
-  default:
-    assert((D.getStorageClass() == VarDecl::None ||
-            D.getStorageClass() == VarDecl::Auto ||
-            D.getStorageClass() == VarDecl::Register) &&
-           "Unknown storage class");
-    return EmitLocalBlockVarDecl(D);
   }
+
+  assert(0 && "Unknown storage class");
 }
 
 llvm::GlobalVariable *

clang/lib/CodeGen/CGExpr.cpp

@@ -632,7 +632,7 @@ LValue CodeGenFunction::EmitDeclRefLValue(const DeclRefExpr *E) {
         isa<ImplicitParamDecl>(VD))) {
     LValue LV;
     bool GCable = VD->hasLocalStorage() && ! VD->getAttr<BlocksAttr>();
-    if (VD->getStorageClass() == VarDecl::Extern) {
+    if (VD->hasExternalStorage()) {
       LV = LValue::MakeAddr(CGM.GetAddrOfGlobalVar(VD),
                             E->getType().getCVRQualifiers(),
                             getContext().getObjCGCAttrKind(E->getType()));

clang/lib/Sema/SemaDecl.cpp

@@ -884,8 +884,7 @@ bool Sema::MergeVarDecl(VarDecl *New, Decl *OldD) {
 
   // C99 6.2.2p4: Check if we have a static decl followed by a non-static.
   if (New->getStorageClass() == VarDecl::Static &&
-      (Old->getStorageClass() == VarDecl::None ||
-       Old->getStorageClass() == VarDecl::Extern)) {
+      (Old->getStorageClass() == VarDecl::None || Old->hasExternalStorage())) {
     Diag(New->getLocation(), diag::err_static_non_static) << New->getDeclName();
     Diag(Old->getLocation(), diag::note_previous_definition);
     return true;
@@ -907,8 +906,12 @@ bool Sema::MergeVarDecl(VarDecl *New, Decl *OldD) {
     Diag(Old->getLocation(), diag::note_previous_definition);
     return true;
   }
+
   // Variables with external linkage are analyzed in FinalizeDeclaratorGroup.
-  if (New->getStorageClass() != VarDecl::Extern && !New->isFileVarDecl() &&
+  
+  // FIXME: The test for external storage here seems wrong? We still
+  // need to check for mismatches.
+  if (!New->hasExternalStorage() && !New->isFileVarDecl() &&
       // Don't complain about out-of-line definitions of static members.
       !(Old->getLexicalDeclContext()->isRecord() &&
         !New->getLexicalDeclContext()->isRecord())) {
@@ -2422,8 +2425,7 @@ void Sema::AddInitializerToDecl(DeclPtrTy dcl, ExprArg init, bool DirectInit) {
   // CheckInitializerTypes may change it.
   QualType DclT = VDecl->getType(), SavT = DclT;
   if (VDecl->isBlockVarDecl()) {
-    VarDecl::StorageClass SC = VDecl->getStorageClass();
-    if (SC == VarDecl::Extern) { // C99 6.7.8p5
+    if (VDecl->hasExternalStorage()) { // C99 6.7.8p5
       Diag(VDecl->getLocation(), diag::err_block_extern_cant_init);
       VDecl->setInvalidDecl();
     } else if (!VDecl->isInvalidDecl()) {
@@ -2434,7 +2436,7 @@ void Sema::AddInitializerToDecl(DeclPtrTy dcl, ExprArg init, bool DirectInit) {
       // C++ 3.6.2p2, allow dynamic initialization of static initializers.
       // Don't check invalid declarations to avoid emitting useless diagnostics.
       if (!getLangOptions().CPlusPlus && !VDecl->isInvalidDecl()) {
-        if (SC == VarDecl::Static) // C99 6.7.8p4.
+        if (VDecl->getStorageClass() == VarDecl::Static) // C99 6.7.8p4.
           CheckForConstantInitializer(Init, DclT);
       }
     }
@@ -2486,7 +2488,7 @@ void Sema::AddInitializerToDecl(DeclPtrTy dcl, ExprArg init, bool DirectInit) {
       }
     }
   } else if (VDecl->isFileVarDecl()) {
-    if (VDecl->getStorageClass() == VarDecl::Extern)
+    if (VDecl->hasExternalStorage())
       Diag(VDecl->getLocation(), diag::warn_extern_init);
     if (!VDecl->isInvalidDecl())
       if (CheckInitializerTypes(Init, DclT, VDecl->getLocation(),
@@ -2529,9 +2531,7 @@ void Sema::ActOnUninitializedDecl(DeclPtrTy dcl) {
     //   function return type, in the declaration of a class member
     //   within its class declaration (9.2), and where the extern
     //   specifier is explicitly used.
-    if (Type->isReferenceType() && 
-        Var->getStorageClass() != VarDecl::Extern &&
-        Var->getStorageClass() != VarDecl::PrivateExtern) {
+    if (Type->isReferenceType() && !Var->hasExternalStorage()) {
       Diag(Var->getLocation(), diag::err_reference_var_requires_init)
         << Var->getDeclName()
         << SourceRange(Var->getLocation(), Var->getLocation());
@@ -2550,9 +2550,7 @@ void Sema::ActOnUninitializedDecl(DeclPtrTy dcl) {
       QualType InitType = Type;
       if (const ArrayType *Array = Context.getAsArrayType(Type))
         InitType = Array->getElementType();
-      if (Var->getStorageClass() != VarDecl::Extern &&
-          Var->getStorageClass() != VarDecl::PrivateExtern &&
-          InitType->isRecordType()) {
+      if (!Var->hasExternalStorage() && InitType->isRecordType()) {
         const CXXConstructorDecl *Constructor = 0;
         if (!RequireCompleteType(Var->getLocation(), InitType, 
                                     diag::err_invalid_incomplete_type_use))
@@ -2593,7 +2591,7 @@ void Sema::ActOnUninitializedDecl(DeclPtrTy dcl) {
     // constructor check.
     if (getLangOptions().CPlusPlus &&
         Context.getCanonicalType(Type).isConstQualified() &&
-        Var->getStorageClass() != VarDecl::Extern)
+        !Var->hasExternalStorage())
       Diag(Var->getLocation(),  diag::err_const_var_requires_init)
         << Var->getName()
         << SourceRange(Var->getLocation(), Var->getLocation());
@@ -2619,8 +2617,7 @@ Sema::DeclGroupPtrTy Sema::FinalizeDeclaratorGroup(Scope *S, DeclPtrTy *Group,
     
     // Block scope. C99 6.7p7: If an identifier for an object is declared with
     // no linkage (C99 6.2.2p6), the type for the object shall be complete...
-    if (IDecl->isBlockVarDecl() && 
-        IDecl->getStorageClass() != VarDecl::Extern) {
+    if (IDecl->isBlockVarDecl() && !IDecl->hasExternalStorage()) {
       if (!IDecl->isInvalidDecl() &&
           RequireCompleteType(IDecl->getLocation(), T, 
                               diag::err_typecheck_decl_incomplete_type))

clang/lib/Sema/SemaDeclObjC.cpp

@@ -1356,8 +1356,7 @@ void Sema::ActOnAtEnd(SourceLocation AtEndLoc, DeclPtrTy classDecl,
       DeclGroupRef DG = allTUVars[i].getAsVal<DeclGroupRef>();
       for (DeclGroupRef::iterator I = DG.begin(), E = DG.end(); I != E; ++I)
         if (VarDecl *VDecl = dyn_cast<VarDecl>(*I)) {
-          if (VDecl->getStorageClass() != VarDecl::Extern &&
-              VDecl->getStorageClass() != VarDecl::PrivateExtern)
+          if (!VDecl->hasExternalStorage())
             Diag(VDecl->getLocation(), diag::err_objc_var_decl_inclass);
         }
     }

clang/test/Sema/private-extern.c

@@ -0,0 +1,89 @@
+// RUN: clang-cc -verify -fsyntax-only %s
+
+static int g0; // expected-note{{previous definition}}
+int g0; // expected-error{{non-static declaration of 'g0' follows static declaration}}
+
+static int g1;
+extern int g1;
+
+static int g2; 
+__private_extern__ int g2;
+
+int g3; // expected-note{{previous definition}}
+static int g3; // expected-error{{static declaration of 'g3' follows non-static declaration}}
+
+extern int g4; // expected-note{{previous definition}}
+static int g4; // expected-error{{static declaration of 'g4' follows non-static declaration}}
+
+__private_extern__ int g5; // expected-note{{previous definition}}
+static int g5; // expected-error{{static declaration of 'g5' follows non-static declaration}}
+
+void f0() {
+  // FIXME: Diagnose this?
+  int g6;
+  extern int g6;
+}
+
+void f1() {
+  // FIXME: Diagnose this?
+  int g7;
+  __private_extern__ int g7;
+}
+
+void f2() {
+  extern int g8; // expected-note{{previous definition}}
+  // FIXME: Improve this diagnostic.
+  int g8; // expected-error{{redefinition of 'g8'}}
+}
+
+void f3() {
+  __private_extern__ int g9; // expected-note{{previous definition}}
+  // FIXME: Improve this diagnostic.
+  int g9; // expected-error{{redefinition of 'g9'}}
+}
+
+void f4() {
+  extern int g10;
+  extern int g10;
+}
+
+void f5() {
+  __private_extern__ int g11;
+  __private_extern__ int g11;
+}
+
+void f6() {
+  // FIXME: Diagnose
+  extern int g12;
+  __private_extern__ int g12;
+}
+
+void f7() {
+  // FIXME: Diagnose
+  __private_extern__ int g13;
+  extern int g13;
+}
+
+struct s0;
+void f8() {
+  extern struct s0 g14;
+  __private_extern__ struct s0 g14;
+}
+struct s0 { int x; };
+
+void f9() {
+  extern int g15 = 0; // expected-error{{'extern' variable cannot have an initializer}}
+  // FIXME: linkage specifier in warning.
+  __private_extern__ int g16 = 0; // expected-error{{'extern' variable cannot have an initializer}}
+}
+
+extern int g17;
+int g17 = 0;
+
+extern int g18 = 0; // expected-warning{{'extern' variable has an initializer}}
+
+__private_extern__ int g19;
+int g19 = 0;
+
+// FIXME: linkage specifier in warning.
+__private_extern__ int g20 = 0; // expected-warning{{'extern' variable has an initializer}}