From 6168bd2323cdf2ff1dc4174c6fc4cd6a479f001b Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Mon, 18 Feb 2013 15:53:43 +0000 Subject: [PATCH] Ensure that the identifier chains have the most recent declaration after module deserialization. This commit introduces a set of related changes to ensure that the declaration that shows up in the identifier chain after deserializing declarations with a given identifier is, in fact, the most recent declaration. The primary change involves waiting until after we deserialize and wire up redeclaration chains before updating the identifier chains. There is a minor optimization in here to avoid recursively deserializing names as part of looking to see whether top-level declarations for a given name exist. A related change that became suddenly more urgent is to property record a merged declaration when an entity first declared in the current translation unit is later deserialized from a module (that had not been loaded at the time of the original declaration). Since we key off the canonical declaration (which is parsed, not from an AST file) for emitted redeclarations, we simply record this as a merged declaration during AST writing and let the readers merge them. Re-fixes , presumably for good this time. llvm-svn: 175447 --- clang/include/clang/Serialization/ASTReader.h | 12 ++----- clang/lib/Serialization/ASTReader.cpp | 49 ++++++++++++++++++-------- clang/lib/Serialization/ASTReaderDecl.cpp | 24 +++++++++++++ clang/lib/Serialization/ASTWriter.cpp | 18 ++++++++-- clang/test/Modules/Inputs/redecl-merge-left.h | 1 + clang/test/Modules/Inputs/redecl-merge-right.h | 3 ++ clang/test/Modules/Inputs/redecl-merge-top.h | 2 ++ clang/test/Modules/redecl-merge.m | 25 ++++++++++++- 8 files changed, 106 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 69a012c..b8943e73 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -792,19 +792,13 @@ private: /// Number of CXX base specifiers currently loaded unsigned NumCXXBaseSpecifiersLoaded; - /// \brief An IdentifierInfo that has been loaded but whose top-level - /// declarations of the same name have not (yet) been loaded. - struct PendingIdentifierInfo { - IdentifierInfo *II; - SmallVector DeclIDs; - }; - /// \brief The set of identifiers that were read while the AST reader was /// (recursively) loading declarations. /// /// The declarations on the identifier chain for these identifiers will be /// loaded once the recursive loading has completed. - std::deque PendingIdentifierInfos; + llvm::MapVector > + PendingIdentifierInfos; /// \brief The generation number of each identifier, which keeps track of /// the last time we loaded information about this identifier. @@ -1582,7 +1576,7 @@ public: void SetIdentifierInfo(unsigned ID, IdentifierInfo *II); void SetGloballyVisibleDecls(IdentifierInfo *II, const SmallVectorImpl &DeclIDs, - bool Nonrecursive = false); + SmallVectorImpl *Decls = 0); /// \brief Report a diagnostic. DiagnosticBuilder Diag(unsigned DiagID); diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index ee558dc..901eeb1 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -5830,8 +5830,8 @@ void ASTReader::InitializeSema(Sema &S) { // Makes sure any declarations that were deserialized "too early" // still get added to the identifier's declaration chains. for (unsigned I = 0, N = PreloadedDecls.size(); I != N; ++I) { - SemaObj->pushExternalDeclIntoScope(PreloadedDecls[I], - PreloadedDecls[I]->getDeclName()); + NamedDecl *ND = cast(PreloadedDecls[I]->getMostRecentDecl()); + SemaObj->pushExternalDeclIntoScope(ND, PreloadedDecls[I]->getDeclName()); } PreloadedDecls.clear(); @@ -6208,28 +6208,32 @@ void ASTReader::SetIdentifierInfo(IdentifierID ID, IdentifierInfo *II) { /// \param DeclIDs the set of declaration IDs with the name @p II that are /// visible at global scope. /// -/// \param Nonrecursive should be true to indicate that the caller knows that -/// this call is non-recursive, and therefore the globally-visible declarations -/// will not be placed onto the pending queue. +/// \param Decls if non-null, this vector will be populated with the set of +/// deserialized declarations. These declarations will not be pushed into +/// scope. void ASTReader::SetGloballyVisibleDecls(IdentifierInfo *II, const SmallVectorImpl &DeclIDs, - bool Nonrecursive) { - if (NumCurrentElementsDeserializing && !Nonrecursive) { - PendingIdentifierInfos.push_back(PendingIdentifierInfo()); - PendingIdentifierInfo &PII = PendingIdentifierInfos.back(); - PII.II = II; - PII.DeclIDs.append(DeclIDs.begin(), DeclIDs.end()); + SmallVectorImpl *Decls) { + if (NumCurrentElementsDeserializing && !Decls) { + PendingIdentifierInfos[II].append(DeclIDs.begin(), DeclIDs.end()); return; } for (unsigned I = 0, N = DeclIDs.size(); I != N; ++I) { NamedDecl *D = cast(GetDecl(DeclIDs[I])); if (SemaObj) { + // If we're simply supposed to record the declarations, do so now. + if (Decls) { + Decls->push_back(D); + continue; + } + // Introduce this declaration into the translation-unit scope // and add it to the declaration chain for this identifier, so // that (unqualified) name lookup will find it. - SemaObj->pushExternalDeclIntoScope(D, II); + NamedDecl *ND = cast(D->getMostRecentDecl()); + SemaObj->pushExternalDeclIntoScope(ND, II); } else { // Queue this declaration so that it will be added to the // translation unit scope and identifier's declaration chain @@ -6989,10 +6993,14 @@ void ASTReader::finishPendingActions() { !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty()) { // If any identifiers with corresponding top-level declarations have // been loaded, load those declarations now. + llvm::DenseMap > TopLevelDecls; while (!PendingIdentifierInfos.empty()) { - SetGloballyVisibleDecls(PendingIdentifierInfos.front().II, - PendingIdentifierInfos.front().DeclIDs, true); - PendingIdentifierInfos.pop_front(); + // FIXME: std::move + IdentifierInfo *II = PendingIdentifierInfos.back().first; + SmallVector DeclIDs = PendingIdentifierInfos.back().second; + PendingIdentifierInfos.erase(II); + + SetGloballyVisibleDecls(II, DeclIDs, &TopLevelDecls[II]); } // Load pending declaration chains. @@ -7002,6 +7010,17 @@ void ASTReader::finishPendingActions() { } PendingDeclChains.clear(); + // Make the most recent of the top-level declarations visible. + for (llvm::DenseMap >::iterator + TLD = TopLevelDecls.begin(), TLDEnd = TopLevelDecls.end(); + TLD != TLDEnd; ++TLD) { + IdentifierInfo *II = TLD->first; + for (unsigned I = 0, N = TLD->second.size(); I != N; ++I) { + NamedDecl *ND = cast(TLD->second[I]->getMostRecentDecl()); + SemaObj->pushExternalDeclIntoScope(ND, II); + } + } + // Load any pending macro definitions. for (unsigned I = 0; I != PendingMacroIDs.size(); ++I) { // FIXME: std::move here diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index ea54939..f4d03cf 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1806,6 +1806,30 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { if (DC->isTranslationUnit() && Reader.SemaObj) { IdentifierResolver &IdResolver = Reader.SemaObj->IdResolver; + + // Temporarily consider the identifier to be up-to-date. We don't want to + // cause additional lookups here. + class UpToDateIdentifierRAII { + IdentifierInfo *II; + bool WasOutToDate; + + public: + explicit UpToDateIdentifierRAII(IdentifierInfo *II) + : II(II), WasOutToDate(false) + { + if (II) { + WasOutToDate = II->isOutOfDate(); + if (WasOutToDate) + II->setOutOfDate(false); + } + } + + ~UpToDateIdentifierRAII() { + if (WasOutToDate) + II->setOutOfDate(true); + } + } UpToDate(Name.getAsIdentifierInfo()); + for (IdentifierResolver::iterator I = IdResolver.begin(Name), IEnd = IdResolver.end(); I != IEnd; ++I) { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 79632b8..8747bc8 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -3141,20 +3141,32 @@ void ASTWriter::WriteRedeclarations() { LocalRedeclChains.push_back(0); // Placeholder for the size. // Collect the set of local redeclarations of this declaration. - for (Decl *Prev = MostRecent; Prev != First; + for (Decl *Prev = MostRecent; Prev != First; Prev = Prev->getPreviousDecl()) { if (!Prev->isFromASTFile()) { AddDeclRef(Prev, LocalRedeclChains); ++Size; } } + + if (!First->isFromASTFile() && Chain) { + Decl *FirstFromAST = MostRecent; + for (Decl *Prev = MostRecent; Prev; Prev = Prev->getPreviousDecl()) { + if (Prev->isFromASTFile()) + FirstFromAST = Prev; + } + + Chain->MergedDecls[FirstFromAST].push_back(getDeclID(First)); + } + LocalRedeclChains[Offset] = Size; // Reverse the set of local redeclarations, so that we store them in // order (since we found them in reverse order). std::reverse(LocalRedeclChains.end() - Size, LocalRedeclChains.end()); - // Add the mapping from the first ID to the set of local declarations. + // Add the mapping from the first ID from the AST to the set of local + // declarations. LocalRedeclarationsInfo Info = { getDeclID(First), Offset }; LocalRedeclsMap.push_back(Info); @@ -3807,8 +3819,8 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, WriteMacroUpdates(); WriteDeclUpdatesBlocks(); WriteDeclReplacementsBlock(); - WriteMergedDecls(); WriteRedeclarations(); + WriteMergedDecls(); WriteObjCCategories(); // Some simple statistics diff --git a/clang/test/Modules/Inputs/redecl-merge-left.h b/clang/test/Modules/Inputs/redecl-merge-left.h index cf07165..d66b4aa 100644 --- a/clang/test/Modules/Inputs/redecl-merge-left.h +++ b/clang/test/Modules/Inputs/redecl-merge-left.h @@ -90,3 +90,4 @@ typedef void funcptr_with_id(int id); @class DeclaredThenLoaded; +void eventually_noreturn2(void); diff --git a/clang/test/Modules/Inputs/redecl-merge-right.h b/clang/test/Modules/Inputs/redecl-merge-right.h index b664ae9..46a16d3 100644 --- a/clang/test/Modules/Inputs/redecl-merge-right.h +++ b/clang/test/Modules/Inputs/redecl-merge-right.h @@ -85,3 +85,6 @@ const int one = ONE; @interface ClassWithDef - (void)method; @end + +void eventually_noreturn(void) __attribute__((noreturn)); +void eventually_noreturn2(void) __attribute__((noreturn)); diff --git a/clang/test/Modules/Inputs/redecl-merge-top.h b/clang/test/Modules/Inputs/redecl-merge-top.h index 690e6df..27e71a7 100644 --- a/clang/test/Modules/Inputs/redecl-merge-top.h +++ b/clang/test/Modules/Inputs/redecl-merge-top.h @@ -16,3 +16,5 @@ struct S2; struct S2; int func1(int); + +void eventually_noreturn(void); diff --git a/clang/test/Modules/redecl-merge.m b/clang/test/Modules/redecl-merge.m index 8937002..e373667 100644 --- a/clang/test/Modules/redecl-merge.m +++ b/clang/test/Modules/redecl-merge.m @@ -1,5 +1,6 @@ // RUN: rm -rf %t -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I %S/Inputs %s -verify -Wno-objc-root-class +// RUN: %clang_cc1 -fmodules -Wreturn-type -fmodules-cache-path=%t -I %S/Inputs %s -verify -Wno-objc-root-class + @class C2; @class C3; @class C3; @@ -8,8 +9,28 @@ typedef struct my_struct_type *my_struct_ref; @protocol P4; @class C3; @class C3; + +int *call_eventually_noreturn(void) { + eventually_noreturn(); +} // expected-warning{{control reaches end of non-void function}} + +int *call_eventually_noreturn2(void) { + eventually_noreturn2(); +} // expected-warning{{control reaches end of non-void function}} + @import redecl_merge_right; +int *call_eventually_noreturn_again(void) { + eventually_noreturn(); +} + +int *call_eventually_noreturn2_again(void) { + // noreturn and non-noreturn functions have different types + eventually_noreturn2(); // expected-error{{call to 'eventually_noreturn2' is ambiguous}} + // expected-note@93{{candidate function}} + // expected-note@90{{candidate function}} +} + @implementation A - (Super*)init { return self; } @end @@ -148,3 +169,5 @@ id p3; // Make sure we don't get conflicts with 'id'. funcptr_with_id fid; id id_global; + + -- 2.7.4