r233261 - [Modules] Make "#pragma weak" undeclared identifiers be tracked

Chandler Carruth chandlerc at gmail.com
Thu Mar 26 01:32:49 PDT 2015


Author: chandlerc
Date: Thu Mar 26 03:32:49 2015
New Revision: 233261

URL: http://llvm.org/viewvc/llvm-project?rev=233261&view=rev
Log:
[Modules] Make "#pragma weak" undeclared identifiers be tracked
deterministically.

This fixes a latent issue where even Clang's Sema (and diagnostics) were
non-deterministic in the face of this pragma. The fix is super simple --
just use a MapVector so we track the order in which these are parsed (or
imported). Especially considering how rare they are, this seems like the
perfect tradeoff. I've also simplified the client code with judicious
use of auto and range based for loops.

I've added some pretty hilarious code to my stress test which now
survives the binary diff without issue.

Modified:
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/Sema.cpp
    cfe/trunk/lib/Sema/SemaDeclAttr.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp
    cfe/trunk/test/Modules/Inputs/stress1/common.h
    cfe/trunk/test/Modules/Inputs/stress1/merge00.h
    cfe/trunk/test/Modules/stress1.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=233261&r1=233260&r2=233261&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Mar 26 03:32:49 2015
@@ -592,7 +592,7 @@ public:
   /// WeakUndeclaredIdentifiers - Identifiers contained in
   /// \#pragma weak before declared. rare. may alias another
   /// identifier, declared or undeclared
-  llvm::DenseMap<IdentifierInfo*,WeakInfo> WeakUndeclaredIdentifiers;
+  llvm::MapVector<IdentifierInfo *, WeakInfo> WeakUndeclaredIdentifiers;
 
   /// ExtnameUndeclaredIdentifiers - Identifiers contained in
   /// \#pragma redefine_extname before declared.  Used in Solaris system headers

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=233261&r1=233260&r2=233261&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Thu Mar 26 03:32:49 2015
@@ -524,14 +524,8 @@ void Sema::LoadExternalWeakUndeclaredIde
 
   SmallVector<std::pair<IdentifierInfo *, WeakInfo>, 4> WeakIDs;
   ExternalSource->ReadWeakUndeclaredIdentifiers(WeakIDs);
-  for (unsigned I = 0, N = WeakIDs.size(); I != N; ++I) {
-    llvm::DenseMap<IdentifierInfo*,WeakInfo>::iterator Pos
-      = WeakUndeclaredIdentifiers.find(WeakIDs[I].first);
-    if (Pos != WeakUndeclaredIdentifiers.end())
-      continue;
-
-    WeakUndeclaredIdentifiers.insert(WeakIDs[I]);
-  }
+  for (auto &WeakID : WeakIDs)
+    WeakUndeclaredIdentifiers.insert(WeakID);
 }
 
 
@@ -694,16 +688,13 @@ void Sema::ActOnEndOfTranslationUnit() {
   }
 
   // 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.
   LoadExternalWeakUndeclaredIdentifiers();
-  for (llvm::DenseMap<IdentifierInfo*,WeakInfo>::iterator
-       I = WeakUndeclaredIdentifiers.begin(),
-       E = WeakUndeclaredIdentifiers.end(); I != E; ++I) {
-    if (I->second.getUsed()) continue;
+  for (auto WeakID : WeakUndeclaredIdentifiers) {
+    if (WeakID.second.getUsed())
+      continue;
 
-    Diag(I->second.getLocation(), diag::warn_weak_identifier_undeclared)
-      << I->first;
+    Diag(WeakID.second.getLocation(), diag::warn_weak_identifier_undeclared)
+        << WeakID.first;
   }
 
   if (LangOpts.CPlusPlus11 &&

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=233261&r1=233260&r2=233261&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Mar 26 03:32:49 2015
@@ -5019,8 +5019,7 @@ void Sema::ProcessPragmaWeak(Scope *S, D
         ND = FD;
     if (ND) {
       if (IdentifierInfo *Id = ND->getIdentifier()) {
-        llvm::DenseMap<IdentifierInfo*,WeakInfo>::iterator I
-          = WeakUndeclaredIdentifiers.find(Id);
+        auto I = WeakUndeclaredIdentifiers.find(Id);
         if (I != WeakUndeclaredIdentifiers.end()) {
           WeakInfo W = I->second;
           DeclApplyPragmaWeak(S, ND, W);

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=233261&r1=233260&r2=233261&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Mar 26 03:32:49 2015
@@ -4359,15 +4359,13 @@ void ASTWriter::WriteASTCore(Sema &SemaR
   // entire table, since later PCH files in a PCH chain are only interested in
   // the results at the end of the chain.
   RecordData WeakUndeclaredIdentifiers;
-  if (!SemaRef.WeakUndeclaredIdentifiers.empty()) {
-    for (llvm::DenseMap<IdentifierInfo*,WeakInfo>::iterator
-         I = SemaRef.WeakUndeclaredIdentifiers.begin(),
-         E = SemaRef.WeakUndeclaredIdentifiers.end(); I != E; ++I) {
-      AddIdentifierRef(I->first, WeakUndeclaredIdentifiers);
-      AddIdentifierRef(I->second.getAlias(), WeakUndeclaredIdentifiers);
-      AddSourceLocation(I->second.getLocation(), WeakUndeclaredIdentifiers);
-      WeakUndeclaredIdentifiers.push_back(I->second.getUsed());
-    }
+  for (auto &WeakUndeclaredIdentifier : SemaRef.WeakUndeclaredIdentifiers) {
+    IdentifierInfo *II = WeakUndeclaredIdentifier.first;
+    WeakInfo &WI = WeakUndeclaredIdentifier.second;
+    AddIdentifierRef(II, WeakUndeclaredIdentifiers);
+    AddIdentifierRef(WI.getAlias(), WeakUndeclaredIdentifiers);
+    AddSourceLocation(WI.getLocation(), WeakUndeclaredIdentifiers);
+    WeakUndeclaredIdentifiers.push_back(WI.getUsed());
   }
 
   // Build a record containing all of the ext_vector declarations.

Modified: cfe/trunk/test/Modules/Inputs/stress1/common.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/stress1/common.h?rev=233261&r1=233260&r2=233261&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/stress1/common.h (original)
+++ cfe/trunk/test/Modules/Inputs/stress1/common.h Thu Mar 26 03:32:49 2015
@@ -35,4 +35,18 @@ struct S01 {};
 struct S02 {};
 }
 
+#pragma weak pragma_weak00
+#pragma weak pragma_weak01
+#pragma weak pragma_weak02
+#pragma weak pragma_weak03
+#pragma weak pragma_weak04
+#pragma weak pragma_weak05
+
+extern "C" int pragma_weak00();
+extern "C" int pragma_weak01();
+extern "C" int pragma_weak02();
+extern "C" int pragma_weak03;
+extern "C" int pragma_weak04;
+extern "C" int pragma_weak05;
+
 #endif

Modified: cfe/trunk/test/Modules/Inputs/stress1/merge00.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/stress1/merge00.h?rev=233261&r1=233260&r2=233261&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/stress1/merge00.h (original)
+++ cfe/trunk/test/Modules/Inputs/stress1/merge00.h Thu Mar 26 03:32:49 2015
@@ -1,6 +1,14 @@
 #ifndef STRESS1_MERGE00_H
 #define STRESS1_MERGE00_H
 
+// These don't match the imported declarations because we import them from
+// modules which are built in isolation of the current header's pragma state
+// much like they are built in isolation of the incoming macro state.
+// FIXME: We should expect warnings here but we can't because verify doesn't
+// work for modules.
+//#pragma weak pragma_weak01 // expected-warning {{weak identifier 'pragma_weak01' never declared}}
+//#pragma weak pragma_weak04 // expected-warning {{weak identifier 'pragma_waek04' never declared}}
+
 #include "m00.h"
 #include "m01.h"
 #include "m02.h"
@@ -8,4 +16,10 @@
 
 inline int g() { return N00::S00('a').method00('b') + (int)N00::S00(42) + function00(42); }
 
+#pragma weak pragma_weak02
+#pragma weak pragma_weak05
+
+extern "C" int pragma_weak02();
+int pragma_weak05;
+
 #endif

Modified: cfe/trunk/test/Modules/stress1.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/stress1.cpp?rev=233261&r1=233260&r2=233261&view=diff
==============================================================================
--- cfe/trunk/test/Modules/stress1.cpp (original)
+++ cfe/trunk/test/Modules/stress1.cpp Thu Mar 26 03:32:49 2015
@@ -98,3 +98,8 @@
 #include "merge00.h"
 
 int f() { return N01::S00('a').method00('b') + (int)N00::S00(42) + function00(42) + g(); }
+
+int f2() {
+  return pragma_weak00() + pragma_weak01() + pragma_weak02() +
+         pragma_weak03 + pragma_weak04 + pragma_weak05;
+}





More information about the cfe-commits mailing list