Explictly track tentative definitions within Sema, then hand those

tentative definitions off to the ASTConsumer at the end of the
translation unit. 

Eliminate CodeGen's internal tracking of tentative definitions, and
instead hook into ASTConsumer::CompleteTentativeDefinition. Also,
tweak the definition-deferal logic for C++, where there are no
tentative definitions.

Fixes <rdar://problem/6808352>, and will make it much easier for
precompiled headers to cope with tentative definitions in the future.

llvm-svn: 69681

clang/include/clang/AST/ASTConsumer.h

@@ -20,6 +20,7 @@ namespace clang {
   class TagDecl;
   class HandleTagDeclDefinition;
   class SemaConsumer; // layering violation required for safe SemaConsumer
+  class VarDecl;
 
 /// ASTConsumer - This is an abstract interface that should be implemented by
 /// clients that read ASTs.  This abstraction layer allows the client to be
@@ -56,6 +57,17 @@ public:
   /// can be defined in declspecs).
   virtual void HandleTagDeclDefinition(TagDecl *D) {}
   
+  /// \brief Callback invoked at the end of a translation unit to
+  /// notify the consumer that the given tentative definition should
+  /// be completed.
+  ///
+  /// The variable declaration itself will be a tentative
+  /// definition. If it had an incomplete array type, its type will
+  /// have already been changed to an array of size 1. However, the
+  /// declaration remains a tentative definition and has not been
+  /// modified by the introduction of an implicit zero initializer.
+  virtual void CompleteTentativeDefinition(VarDecl *D) {}
+
   /// PrintStats - If desired, print any statistics.
   virtual void PrintStats() {
   }

clang/lib/AST/Decl.cpp

@@ -291,7 +291,8 @@ bool VarDecl::isTentativeDefinition(ASTContext &Context) const {
   if (!isFileVarDecl() || Context.getLangOptions().CPlusPlus)
     return false;
 
-  return (!getInit() &&
+  const VarDecl *Def = 0;
+  return (!getDefinition(Def) &&
           (getStorageClass() == None || getStorageClass() == Static));
 }
 

clang/lib/CodeGen/CodeGenModule.cpp

@@ -424,13 +424,6 @@ void CodeGenModule::EmitDeferred() {
     // Otherwise, emit the definition and move on to the next one.
     EmitGlobalDefinition(D);
   }
-
-  // Emit any tentative definitions, in reverse order so the most
-  // important (merged) decl will be seen and emitted first.
-  for (std::vector<const VarDecl*>::reverse_iterator 
-         it = TentativeDefinitions.rbegin(), ie = TentativeDefinitions.rend(); 
-       it != ie; ++it)
-    EmitTentativeDefinition(*it);
 }
 
 /// EmitAnnotateAttr - Generate the llvm::ConstantStruct which contains the 
@@ -502,6 +495,7 @@ bool CodeGenModule::MayDeferGeneration(const ValueDecl *Global) {
   
   const VarDecl *VD = cast<VarDecl>(Global);
   assert(VD->isFileVarDecl() && "Invalid decl");
+
   return VD->getStorageClass() == VarDecl::Static;
 }
 
@@ -520,16 +514,14 @@ void CodeGenModule::EmitGlobal(const ValueDecl *Global) {
     const VarDecl *VD = cast<VarDecl>(Global);
     assert(VD->isFileVarDecl() && "Cannot emit local var decl as global.");
 
-    // If this isn't a definition, defer code generation.
-    if (!VD->getInit()) {
-      // If this is a tentative definition, remember it so that we can
-      // emit the common definition if needed. It is important to
-      // defer tentative definitions, since they may have incomplete
-      // type.
-      if (!VD->hasExternalStorage())
-        TentativeDefinitions.push_back(VD);
+    // In C++, if this is marked "extern", defer code generation.
+    if (getLangOptions().CPlusPlus && 
+        VD->getStorageClass() == VarDecl::Extern && !VD->getInit())
+      return;
+
+    // In C, if this isn't a definition, defer code generation.
+    if (!getLangOptions().CPlusPlus && !VD->getInit())
       return;
-    }
   }
 
   // Defer code generation when possible if this is a static definition, inline
@@ -727,6 +719,9 @@ void CodeGenModule::EmitTentativeDefinition(const VarDecl *D) {
   // See if we have already defined this (as a variable), if so we do
   // not need to do anything.
   llvm::GlobalValue *GV = GlobalDeclMap[getMangledName(D)];
+  if (!GV && MayDeferGeneration(D)) // this variable was never referenced
+    return;
+
   if (llvm::GlobalVariable *Var = dyn_cast_or_null<llvm::GlobalVariable>(GV))
     if (Var->hasInitializer())
       return;

clang/lib/CodeGen/CodeGenModule.h

@@ -119,14 +119,6 @@ class CodeGenModule : public BlockModule {
   /// is done.
   std::vector<const ValueDecl*> DeferredDeclsToEmit;
 
-  /// TentativeDefinitions - A list of declarations which are
-  /// tentative definitions. Code generation for these must be
-  /// deferred because they are allowed to have incomplete type when
-  /// they are seen. This also allows us to avoid generating an extra
-  /// common definiton in situations where the tentative definition is
-  /// followed by an actual definition.
-  std::vector<const VarDecl*> TentativeDefinitions;
-  
   /// LLVMUsed - List of global values which are required to be
   /// present in the object file; bitcast to i8*. This is used for
   /// forcing visibility of symbols which may otherwise be optimized
@@ -339,6 +331,8 @@ public:
                                     CXXCtorType Type);
   const char *getMangledCXXDtorName(const CXXDestructorDecl *D, 
                                     CXXDtorType Type);
+
+  void EmitTentativeDefinition(const VarDecl *D);
   
   enum GVALinkage {
     GVA_Internal,
@@ -382,7 +376,6 @@ private:
   void EmitGlobalDefinition(const ValueDecl *D);
 
   void EmitGlobalFunctionDefinition(const FunctionDecl *D);
-  void EmitTentativeDefinition(const VarDecl *D);
   void EmitGlobalVarDefinition(const VarDecl *D);
   void EmitAliasDefinition(const ValueDecl *D);
   void EmitObjCPropertyImplementations(const ObjCImplementationDecl *D);

clang/lib/CodeGen/ModuleBuilder.cpp

@@ -83,6 +83,13 @@ namespace {
       if (Builder)
         Builder->Release();
     };
+
+    virtual void CompleteTentativeDefinition(VarDecl *D) {
+      if (Diags.hasErrorOccurred())
+        return;
+
+      Builder->EmitTentativeDefinition(D);
+    }
   };
 }
 

clang/lib/Sema/Sema.cpp

@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "Sema.h"
+#include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/Expr.h"
@@ -232,41 +233,39 @@ void Sema::ActOnEndOfTranslationUnit() {
   //   translation unit contains a file scope declaration of that
   //   identifier, with the composite type as of the end of the
   //   translation unit, with an initializer equal to 0.
-  if (!getLangOptions().CPlusPlus) {
-    // Note: we traverse the scope's list of declarations rather than
-    // the DeclContext's list, because we only want to see the most
-    // recent declaration of each identifier.
-    for (Scope::decl_iterator I = TUScope->decl_begin(),
-         IEnd = TUScope->decl_end();
-         I != IEnd; ++I) {
-      Decl *D = (*I).getAs<Decl>();
-      if (D->isInvalidDecl())
-        continue;
+  for (llvm::DenseMap<DeclarationName, VarDecl *>::iterator 
+         D = TentativeDefinitions.begin(),
+         DEnd = TentativeDefinitions.end();
+       D != DEnd; ++D) {
+    VarDecl *VD = D->second;
 
-      if (VarDecl *VD = dyn_cast<VarDecl>(D)) {
-        if (VD->isTentativeDefinition(Context)) {
-          if (const IncompleteArrayType *ArrayT 
-                = Context.getAsIncompleteArrayType(VD->getType())) {
-            if (RequireCompleteType(VD->getLocation(), 
-                                    ArrayT->getElementType(),
-                                 diag::err_tentative_def_incomplete_type_arr))
-              VD->setInvalidDecl();
-            else {
-              // Set the length of the array to 1 (C99 6.9.2p5).
-              Diag(VD->getLocation(),  diag::warn_tentative_incomplete_array);
-              llvm::APInt One(Context.getTypeSize(Context.getSizeType()), 
-                              true);
-              QualType T 
-                = Context.getConstantArrayType(ArrayT->getElementType(),
-                                               One, ArrayType::Normal, 0);
-              VD->setType(T);
-            }
-          } else if (RequireCompleteType(VD->getLocation(), VD->getType(), 
-                                    diag::err_tentative_def_incomplete_type))
-            VD->setInvalidDecl();
-        }
+    if (VD->isInvalidDecl() || !VD->isTentativeDefinition(Context))
+      continue;
+
+    if (const IncompleteArrayType *ArrayT 
+        = Context.getAsIncompleteArrayType(VD->getType())) {
+      if (RequireCompleteType(VD->getLocation(), 
+                              ArrayT->getElementType(),
+                              diag::err_tentative_def_incomplete_type_arr))
+        VD->setInvalidDecl();
+      else {
+        // Set the length of the array to 1 (C99 6.9.2p5).
+        Diag(VD->getLocation(),  diag::warn_tentative_incomplete_array);
+        llvm::APInt One(Context.getTypeSize(Context.getSizeType()), 
+                        true);
+        QualType T 
+          = Context.getConstantArrayType(ArrayT->getElementType(),
+                                         One, ArrayType::Normal, 0);
+        VD->setType(T);
       }
-    }
+    } else if (RequireCompleteType(VD->getLocation(), VD->getType(), 
+                                   diag::err_tentative_def_incomplete_type))
+      VD->setInvalidDecl();
+
+    // Notify the consumer that we've completed a tentative definition.
+    if (!VD->isInvalidDecl())
+      Consumer.CompleteTentativeDefinition(VD);
+
   }
 }
 

clang/lib/Sema/Sema.h

@@ -233,6 +233,14 @@ public:
   ///     not visible.
   llvm::DenseMap<DeclarationName, NamedDecl *> LocallyScopedExternalDecls;
 
+  /// \brief The set of tentative declarations seen so far in this
+  /// translation unit for which no definition has been seen.
+  ///
+  /// The tentative declarations are indexed by the name of the
+  /// declaration, and only the most recent tentative declaration for
+  /// a given variable will be recorded here.
+  llvm::DenseMap<DeclarationName, VarDecl *> TentativeDefinitions;
+
   IdentifierResolver IdResolver;
 
   // Enum values used by KnownFunctionIDs (see below).

clang/lib/Sema/SemaDecl.cpp

@@ -2545,6 +2545,18 @@ void Sema::AddInitializerToDecl(DeclPtrTy dcl, ExprArg init, bool DirectInit) {
     
   // Attach the initializer to the decl.
   VDecl->setInit(Init);
+
+  // If the previous declaration of VDecl was a tentative definition,
+  // remove it from the set of tentative definitions.
+  if (VDecl->getPreviousDeclaration() &&
+      VDecl->getPreviousDeclaration()->isTentativeDefinition(Context)) {
+    llvm::DenseMap<DeclarationName, VarDecl *>::iterator Pos 
+      = TentativeDefinitions.find(VDecl->getDeclName());
+    assert(Pos != TentativeDefinitions.end() && 
+           "Unrecorded tentative definition?");
+    TentativeDefinitions.erase(Pos);
+  }
+
   return;
 }
 
@@ -2557,6 +2569,11 @@ void Sema::ActOnUninitializedDecl(DeclPtrTy dcl) {
 
   if (VarDecl *Var = dyn_cast<VarDecl>(RealDecl)) {
     QualType Type = Var->getType();
+
+    // Record tentative definitions.
+    if (Var->isTentativeDefinition(Context))
+      TentativeDefinitions[Var->getDeclName()] = Var;
+
     // C++ [dcl.init.ref]p3:
     //   The initializer can be omitted for a reference only in a
     //   parameter declaration (8.3.5), in the declaration of a

clang/test/PCH/external-defs.h

@@ -14,5 +14,4 @@ static int z;
 int incomplete_array[];
 int incomplete_array2[];
 
-// FIXME: CodeGen problems prevents this from working (<rdar://problem/6762287>)
-// struct S s;
+struct S s;

clang/test/Sema/init.c

@@ -74,8 +74,7 @@ int sym_fw1a_scr[] = {
 };
 
 // PR3001
-struct s1 s2 = { // expected-error{{tentative definition has type 'struct s1' that is never completed}} \
-  // expected-note{{forward declaration of 'struct s1'}}
+struct s1 s2 = {
     .a = sizeof(struct s3), // expected-error {{invalid application of 'sizeof'}} \
                             // expected-note{{forward declaration of 'struct s3'}}
     .b = bogus // expected-error {{use of undeclared identifier 'bogus'}}

clang/test/Sema/tentative-decls.c

@@ -57,3 +57,9 @@ void func2(void)
   extern double *p;
 }
 
+// <rdar://problem/6808352>
+static int a0[];
+static int b0;
+
+static int a0[] = { 4 };
+static int b0 = 5;

clang/tools/clang-cc/Backend.cpp

@@ -158,6 +158,10 @@ namespace {
                                      "LLVM IR generation of declaration");
       Gen->HandleTagDeclDefinition(D);
     }
+
+    virtual void CompleteTentativeDefinition(VarDecl *D) {
+      Gen->CompleteTentativeDefinition(D);
+    }
   };  
 }