[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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@232455 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 96c16ea10ee4ebc6b4c0b2712bcbe3c1830d1bc6..83882a836939f896f5a07938724f43c26fb75118 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -1198,13 +1198,13 @@ void ASTDeclReader::VisitNamespaceDecl(NamespaceDecl *D) {
   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(NamespaceDecl *D) {
   }
 
   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(Redeclarable<T> *DBase, T *Existing,
     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(Redeclarable<T> *DBase, T *Existing,
           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)

diff --git a/test/Modules/Inputs/anon-namespace/a.h b/test/Modules/Inputs/anon-namespace/a.h
new file mode 100644 (file)
index 0000000..fe71f40
--- /dev/null
+++ b/test/Modules/Inputs/anon-namespace/a.h
@@ -0,0 +1 @@
+namespace N {}

diff --git a/test/Modules/Inputs/anon-namespace/b1.h b/test/Modules/Inputs/anon-namespace/b1.h
new file mode 100644 (file)
index 0000000..480e8e4
--- /dev/null
+++ b/test/Modules/Inputs/anon-namespace/b1.h
@@ -0,0 +1,2 @@
+namespace N {}
+namespace N { namespace {} }

diff --git a/test/Modules/Inputs/anon-namespace/b2.h b/test/Modules/Inputs/anon-namespace/b2.h
new file mode 100644 (file)
index 0000000..8e7535f
--- /dev/null
+++ b/test/Modules/Inputs/anon-namespace/b2.h
@@ -0,0 +1,2 @@
+#include "a.h"
+namespace N {}

diff --git a/test/Modules/Inputs/anon-namespace/c.h b/test/Modules/Inputs/anon-namespace/c.h
new file mode 100644 (file)
index 0000000..fe71f40
--- /dev/null
+++ b/test/Modules/Inputs/anon-namespace/c.h
@@ -0,0 +1 @@
+namespace N {}

diff --git a/test/Modules/Inputs/anon-namespace/module.modulemap b/test/Modules/Inputs/anon-namespace/module.modulemap
new file mode 100644 (file)
index 0000000..3d390d2
--- /dev/null
+++ b/test/Modules/Inputs/anon-namespace/module.modulemap
@@ -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 * }

diff --git a/test/Modules/anon-namespace.cpp b/test/Modules/anon-namespace.cpp
new file mode 100644 (file)
index 0000000..6c085eb
--- /dev/null
+++ b/test/Modules/anon-namespace.cpp
@@ -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;