r232455 - [modules] Fix bug where an anonymous namespace could cause the containing

[Richard Smith]

Author: rsmith
Date: Mon Mar 16 21:23:11 2015
New Revision: 232455

URL: http://llvm.org/viewvc/llvm-project?rev=232455&view=rev
Log:
[modules] Fix bug where an anonymous namespace could cause the containing
namespace to not merge properly.

We have an invariant here: after a declaration reads its canonical declaration,
it can assume the canonical declaration is fully merged. This invariant can be
violated if deserializing some declaration triggers the deserialization of a
later declaration, because that later declaration can in turn deserialize a
redeclaration of that first declaration before it is fully merged.

The anonymous namespace for a namespace gets stored with the first declaration
of that namespace, which may be before its parent namespace, so defer loading
it until after we've finished merging the surrounding namespace.

Added:
    cfe/trunk/test/Modules/Inputs/anon-namespace/
    cfe/trunk/test/Modules/Inputs/anon-namespace/a.h
    cfe/trunk/test/Modules/Inputs/anon-namespace/b1.h
    cfe/trunk/test/Modules/Inputs/anon-namespace/b2.h
    cfe/trunk/test/Modules/Inputs/anon-namespace/c.h
    cfe/trunk/test/Modules/Inputs/anon-namespace/module.modulemap
    cfe/trunk/test/Modules/anon-namespace.cpp
Modified:
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=232455&r1=232454&r2=232455&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Mon Mar 16 21:23:11 2015
@@ -1198,13 +1198,13 @@ void ASTDeclReader::VisitNamespaceDecl(N
   D->LocStart = ReadSourceLocation(Record, Idx);
   D->RBraceLoc = ReadSourceLocation(Record, Idx);
 
+  // Defer loading the anonymous namespace until we've finished merging
+  // this namespace; loading it might load a later declaration of the
+  // same namespace, and we have an invariant that older declarations
+  // get merged before newer ones try to merge.
+  GlobalDeclID AnonNamespace = 0;
   if (Redecl.getFirstID() == ThisDeclID) {
-    // Each module has its own anonymous namespace, which is disjoint from
-    // any other module's anonymous namespaces, so don't attach the anonymous
-    // namespace at all.
-    NamespaceDecl *Anon = ReadDeclAs<NamespaceDecl>(Record, Idx);
-    if (F.Kind != MK_ImplicitModule && F.Kind != MK_ExplicitModule)
-      D->setAnonymousNamespace(Anon);
+    AnonNamespace = ReadDeclID(Record, Idx);
   } else {
     // Link this namespace back to the first declaration, which has already
     // been deserialized.
@@ -1212,6 +1212,15 @@ void ASTDeclReader::VisitNamespaceDecl(N
   }
 
   mergeRedeclarable(D, Redecl);
+
+  if (AnonNamespace) {
+    // Each module has its own anonymous namespace, which is disjoint from
+    // any other module's anonymous namespaces, so don't attach the anonymous
+    // namespace at all.
+    NamespaceDecl *Anon = cast<NamespaceDecl>(Reader.GetDecl(AnonNamespace));
+    if (F.Kind != MK_ImplicitModule && F.Kind != MK_ExplicitModule)
+      D->setAnonymousNamespace(Anon);
+  }
 }
 
 void ASTDeclReader::VisitNamespaceAliasDecl(NamespaceAliasDecl *D) {
@@ -2196,6 +2205,8 @@ void ASTDeclReader::mergeRedeclarable(Re
     D->RedeclLink = Redeclarable<T>::PreviousDeclLink(ExistingCanon);
 
     // When we merge a namespace, update its pointer to the first namespace.
+    // We cannot have loaded any redeclarations of this declaration yet, so
+    // there's nothing else that needs to be updated.
     if (auto *Namespace = dyn_cast<NamespaceDecl>(D))
       Namespace->AnonOrFirstNamespaceAndInline.setPointer(
           assert_cast<NamespaceDecl*>(ExistingCanon));
@@ -2206,9 +2217,7 @@ void ASTDeclReader::mergeRedeclarable(Re
           DTemplate, assert_cast<RedeclarableTemplateDecl*>(ExistingCanon),
           TemplatePatternID);
 
-    // If this declaration was the canonical declaration, make a note of
-    // that. We accept the linear algorithm here because the number of
-    // unique canonical declarations of an entity should always be tiny.
+    // If this declaration was the canonical declaration, make a note of that.
     if (DCanon == D) {
       Reader.MergedDecls[ExistingCanon].push_back(Redecl.getFirstID());
       if (Reader.PendingDeclChainsKnown.insert(ExistingCanon).second)

Added: cfe/trunk/test/Modules/Inputs/anon-namespace/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/anon-namespace/a.h?rev=232455&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/anon-namespace/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/anon-namespace/a.h Mon Mar 16 21:23:11 2015
@@ -0,0 +1 @@
+namespace N {}

Added: cfe/trunk/test/Modules/Inputs/anon-namespace/b1.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/anon-namespace/b1.h?rev=232455&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/anon-namespace/b1.h (added)
+++ cfe/trunk/test/Modules/Inputs/anon-namespace/b1.h Mon Mar 16 21:23:11 2015
@@ -0,0 +1,2 @@
+namespace N {}
+namespace N { namespace {} }

Added: cfe/trunk/test/Modules/Inputs/anon-namespace/b2.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/anon-namespace/b2.h?rev=232455&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/anon-namespace/b2.h (added)
+++ cfe/trunk/test/Modules/Inputs/anon-namespace/b2.h Mon Mar 16 21:23:11 2015
@@ -0,0 +1,2 @@
+#include "a.h"
+namespace N {}

Added: cfe/trunk/test/Modules/Inputs/anon-namespace/c.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/anon-namespace/c.h?rev=232455&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/anon-namespace/c.h (added)
+++ cfe/trunk/test/Modules/Inputs/anon-namespace/c.h Mon Mar 16 21:23:11 2015
@@ -0,0 +1 @@
+namespace N {}

Added: cfe/trunk/test/Modules/Inputs/anon-namespace/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/anon-namespace/module.modulemap?rev=232455&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/anon-namespace/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/anon-namespace/module.modulemap Mon Mar 16 21:23:11 2015
@@ -0,0 +1,3 @@
+module a { header "a.h" export * }
+module b { module b1 { header "b1.h" export * } module b2 { header "b2.h" export * } }
+module c { header "c.h" export * }

Added: cfe/trunk/test/Modules/anon-namespace.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/anon-namespace.cpp?rev=232455&view=auto
==============================================================================
--- cfe/trunk/test/Modules/anon-namespace.cpp (added)
+++ cfe/trunk/test/Modules/anon-namespace.cpp Mon Mar 16 21:23:11 2015
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/anon-namespace -verify %s
+// expected-no-diagnostics
+#include "b1.h"
+#include "c.h"
+using namespace N;