Merge 81236 from mainline.

Fix PR4922, where Sema would complete tentative definitions in nondeterminstic
order because it was doing so while iterating over a densemap.

There are still similar problems in other places, for example
WeakUndeclaredIdentifiers is still written to the PCH file in a nondeterminstic
order, and we emit warnings about #pragma weak in nondeterminstic order.

llvm-svn: 81280

5 files changed, 53 insertions, 37 deletions

diff --git a/clang/lib/Frontend/PCHReader.cpp b/clang/lib/Frontend/PCHReader.cpp
index b368124f4f66..3b5ee4454a64 100644
--- a/clang/lib/Frontend/PCHReader.cpp
+++ b/clang/lib/Frontend/PCHReader.cpp
@@ -2215,6 +2215,7 @@ void PCHReader::InitializeSema(Sema &S) {
   for (unsigned I = 0, N = TentativeDefinitions.size(); I != N; ++I) {
     VarDecl *Var = cast<VarDecl>(GetDecl(TentativeDefinitions[I]));
     SemaObj->TentativeDefinitions[Var->getDeclName()] = Var;
+    SemaObj->TentativeDefinitionList.push_back(Var->getDeclName());
   }
 
   // If there were any locally-scoped external declarations,
diff --git a/clang/lib/Frontend/PCHWriter.cpp b/clang/lib/Frontend/PCHWriter.cpp
index 48a1c9d5e3a3..db3cb3a38b8e 100644
--- a/clang/lib/Frontend/PCHWriter.cpp
+++ b/clang/lib/Frontend/PCHWriter.cpp
@@ -1799,19 +1799,22 @@ void PCHWriter::WritePCH(Sema &SemaRef, MemorizeStatCalls *StatCalls,
       getIdentifierRef(&Table.get(BuiltinNames[I]));
   }
 
-  // Build a record containing all of the tentative definitions in
-  // this header file. Generally, this record will be empty.
+  // Build a record containing all of the tentative definitions in this file, in
+  // TentativeDefinitionList order.  Generally, this record will be empty for
+  // headers.
   RecordData TentativeDefinitions;
-  for (llvm::DenseMap<DeclarationName, VarDecl *>::iterator 
-         TD = SemaRef.TentativeDefinitions.begin(),
-         TDEnd = SemaRef.TentativeDefinitions.end();
-       TD != TDEnd; ++TD)
-    AddDeclRef(TD->second, TentativeDefinitions);
+  for (unsigned i = 0, e = SemaRef.TentativeDefinitionList.size(); i != e; ++i){
+    VarDecl *VD =
+      SemaRef.TentativeDefinitions.lookup(SemaRef.TentativeDefinitionList[i]);
+    if (VD) AddDeclRef(VD, TentativeDefinitions);
+  }
 
   // Build a record containing all of the locally-scoped external
   // declarations in this header file. Generally, this record will be
   // empty.
   RecordData LocallyScopedExternalDecls;
+  // FIXME: This is filling in the PCH file in densemap order which is
+  // nondeterminstic!
   for (llvm::DenseMap<DeclarationName, NamedDecl *>::iterator 
          TD = SemaRef.LocallyScopedExternalDecls.begin(),
          TDEnd = SemaRef.LocallyScopedExternalDecls.end();
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 5425c4fb5abc..92af38695526 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -247,13 +247,16 @@ void Sema::ActOnEndOfTranslationUnit() {
   // template instantiations earlier.
   PerformPendingImplicitInstantiations();
   
-  // check for #pragma weak identifiers that were never declared
+  // Check for #pragma weak identifiers that were never declared
+  // FIXME: This will cause diagnostics to be emitted in a non-determinstic
+  // order!  Iterating over a densemap like this is bad.
   for (llvm::DenseMap<IdentifierInfo*,WeakInfo>::iterator
-        I = WeakUndeclaredIdentifiers.begin(),
-        E = WeakUndeclaredIdentifiers.end(); I != E; ++I) {
-      if (!I->second.getUsed())
-        Diag(I->second.getLocation(), diag::warn_weak_identifier_undeclared)
-          << I->first;
+       I = WeakUndeclaredIdentifiers.begin(),
+       E = WeakUndeclaredIdentifiers.end(); I != E; ++I) {
+    if (I->second.getUsed()) continue;
+    
+    Diag(I->second.getLocation(), diag::warn_weak_identifier_undeclared)
+      << I->first;
   }
 
   if (!CompleteTranslationUnit)
@@ -270,31 +273,30 @@ 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.
-  for (llvm::DenseMap<DeclarationName, VarDecl *>::iterator 
-         D = TentativeDefinitions.begin(),
-         DEnd = TentativeDefinitions.end();
-       D != DEnd; ++D) {
-    VarDecl *VD = D->second;
-
-    if (VD->isInvalidDecl() || !VD->isTentativeDefinition(Context))
+  for (unsigned i = 0, e = TentativeDefinitionList.size(); i != e; ++i) {
+    VarDecl *VD = TentativeDefinitions.lookup(TentativeDefinitionList[i]);
+    
+    // If the tentative definition was completed, it will be in the list, but
+    // not the map.
+    if (VD == 0 || 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))
+                              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.getConstantArrayWithoutExprType(ArrayT->getElementType(),
-                                                    One, ArrayType::Normal, 0);
-        VD->setType(T);
+        continue;
       }
+      
+      // 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.getConstantArrayWithoutExprType(ArrayT->getElementType(),
+                                                  One, ArrayType::Normal, 0);
+      VD->setType(T);
     } else if (RequireCompleteType(VD->getLocation(), VD->getType(), 
                                    diag::err_tentative_def_incomplete_type))
       VD->setInvalidDecl();
diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h
index 5feee650cd43..ca96f7e84841 100644
--- a/clang/lib/Sema/Sema.h
+++ b/clang/lib/Sema/Sema.h
@@ -265,6 +265,7 @@ public:
   /// declaration, and only the most recent tentative declaration for
   /// a given variable will be recorded here.
   llvm::DenseMap<DeclarationName, VarDecl *> TentativeDefinitions;
+  std::vector<DeclarationName> TentativeDefinitionList;
 
   /// WeakUndeclaredIdentifiers - Identifiers contained in
   /// #pragma weak before declared. rare. may alias another
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 6668aeaac559..2db8208d7245 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3182,11 +3182,8 @@ void Sema::AddInitializerToDecl(DeclPtrTy dcl, ExprArg init, bool DirectInit) {
   // 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);
+    bool Deleted = TentativeDefinitions.erase(VDecl->getDeclName());
+    assert(Deleted && "Unrecorded tentative definition?"); Deleted=Deleted;
   }
 
   return;
@@ -3204,8 +3201,20 @@ void Sema::ActOnUninitializedDecl(DeclPtrTy dcl,
     QualType Type = Var->getType();
 
     // Record tentative definitions.
-    if (Var->isTentativeDefinition(Context))
-      TentativeDefinitions[Var->getDeclName()] = Var;
+    if (Var->isTentativeDefinition(Context)) {
+      std::pair<llvm::DenseMap<DeclarationName, VarDecl *>::iterator, bool>
+        InsertPair = 
+           TentativeDefinitions.insert(std::make_pair(Var->getDeclName(), Var));
+      
+      // Keep the latest definition in the map.  If we see 'int i; int i;' we
+      // want the second one in the map.
+      InsertPair.first->second = Var;
+
+      // However, for the list, we don't care about the order, just make sure
+      // that there are no dupes for a given declaration name.
+      if (InsertPair.second)
+        TentativeDefinitionList.push_back(Var->getDeclName());
+    }
 
     // C++ [dcl.init.ref]p3:
     //   The initializer can be omitted for a reference only in a