From: Richard Smith Date: Tue, 17 Mar 2015 02:23:11 +0000 (+0000) Subject: [modules] Fix bug where an anonymous namespace could cause the containing X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e146d8df5bebd293af10c6513319106f0b3713f1;p=clang [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 96c16ea10e..83882a8369 100644 --- 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(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(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 *DBase, T *Existing, D->RedeclLink = Redeclarable::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(D)) Namespace->AnonOrFirstNamespaceAndInline.setPointer( assert_cast(ExistingCanon)); @@ -2206,9 +2217,7 @@ void ASTDeclReader::mergeRedeclarable(Redeclarable *DBase, T *Existing, DTemplate, assert_cast(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 index 0000000000..fe71f4004e --- /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 index 0000000000..480e8e46a9 --- /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 index 0000000000..8e7535f362 --- /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 index 0000000000..fe71f4004e --- /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 index 0000000000..3d390d2d4e --- /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 index 0000000000..6c085ebc88 --- /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;