r231424 - [modules] Rework merging of redeclaration chains on module import.
Argyrios Kyrtzidis
kyrtzidis at apple.com
Sat Mar 7 08:21:44 PST 2015
Hi Richard,
r231424 changed behavior of Sema::LookupVisibleDecls(). It now passes all re-declarations to the VisibleDeclConsumer, while it previously only passed the presumed ‘visible’ one which was the latest.
For example, If I have this code in a module header:
----------------
struct Foo; // #1
struct Foo; // #2
struct Foo { // #3
int x;
};
void callme(void); // #4
void callme(void); // #5
void callme(void); // #6
----------------
LookupVisibleDecls() would previously pass only #3 and #6. Now the behavior is like this:
- It passes all Foo’s #1-3#. For #2 and #3 it also sets ‘Hiding’ to #1.
- It passes all callme’s as well, #4-#6. ‘Hiding’ is never set for any of them.
Is this behavior change expected ?
> On Mar 5, 2015, at 3:24 PM, Richard Smith <richard-llvm at metafoo.co.uk> wrote:
>
> Author: rsmith
> Date: Thu Mar 5 17:24:12 2015
> New Revision: 231424
>
> URL: http://llvm.org/viewvc/llvm-project?rev=231424&view=rev
> Log:
> [modules] Rework merging of redeclaration chains on module import.
>
> We used to save out and eagerly load a (potentially huge) table of merged
> formerly-canonical declarations when we loaded each module. This was extremely
> inefficient in the presence of large amounts of merging, and didn't actually
> save any merging lookup work, because we still needed to perform name lookup to
> check that our merged declaration lists were complete. This also resulted in a
> loss of laziness -- even if we only needed an early declaration of an entity, we
> would eagerly pull in all declarations that had been merged into it regardless.
>
> We now store the relevant fragments of the table within the declarations
> themselves. In detail:
>
> * The first declaration of each entity within a module stores a list of first
> declarations from imported modules that are merged into it.
> * Loading that declaration pre-loads those other entities, so that they appear
> earlier within the redeclaration chain.
> * The name lookup tables list the most recent local lookup result, if there
> is one, or all directly-imported lookup results if not.
>
> Modified:
> cfe/trunk/include/clang/AST/DeclContextInternals.h
> cfe/trunk/include/clang/Serialization/ASTBitCodes.h
> cfe/trunk/include/clang/Serialization/ASTReader.h
> cfe/trunk/include/clang/Serialization/ASTWriter.h
> cfe/trunk/lib/AST/Decl.cpp
> cfe/trunk/lib/Sema/IdentifierResolver.cpp
> cfe/trunk/lib/Sema/SemaLookup.cpp
> cfe/trunk/lib/Serialization/ASTReader.cpp
> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> cfe/trunk/lib/Serialization/ASTWriter.cpp
> cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
> cfe/trunk/test/Modules/linkage-merge.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclContextInternals.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclContextInternals.h?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclContextInternals.h (original)
> +++ cfe/trunk/include/clang/AST/DeclContextInternals.h Thu Mar 5 17:24:12 2015
> @@ -78,17 +78,6 @@ public:
> return getAsVectorAndHasExternal().getPointer();
> }
>
> - bool hasLocalDecls() const {
> - if (NamedDecl *Singleton = getAsDecl()) {
> - return !Singleton->isFromASTFile();
> - } else if (DeclsTy *Vec = getAsVector()) {
> - for (auto *D : *Vec)
> - if (!D->isFromASTFile())
> - return true;
> - }
> - return false;
> - }
> -
> bool hasExternalDecls() const {
> return getAsVectorAndHasExternal().getInt();
> }
>
> Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTBitCodes.h?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Thu Mar 5 17:24:12 2015
> @@ -515,8 +515,7 @@ namespace clang {
> /// imported by the AST file.
> IMPORTED_MODULES = 43,
>
> - /// \brief Record code for the set of merged declarations in an AST file.
> - MERGED_DECLARATIONS = 44,
> + // ID 40 used to be a table of merged canonical declarations.
>
> /// \brief Record code for the array of redeclaration chains.
> ///
>
> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Thu Mar 5 17:24:12 2015
> @@ -965,13 +965,13 @@ private:
> /// \brief The list of redeclaration chains that still need to be
> /// reconstructed.
> ///
> - /// Each element is the global declaration ID of the first declaration in
> - /// the chain. Elements in this vector should be unique; use
> + /// Each element is the canonical declaration of the chain.
> + /// Elements in this vector should be unique; use
> /// PendingDeclChainsKnown to ensure uniqueness.
> - SmallVector<serialization::DeclID, 16> PendingDeclChains;
> + SmallVector<Decl *, 16> PendingDeclChains;
>
> /// \brief Keeps track of the elements added to PendingDeclChains.
> - llvm::SmallSet<serialization::DeclID, 16> PendingDeclChainsKnown;
> + llvm::SmallSet<Decl *, 16> PendingDeclChainsKnown;
>
> /// \brief The list of canonical declarations whose redeclaration chains
> /// need to be marked as incomplete once we're done deserializing things.
> @@ -1031,26 +1031,6 @@ private:
> /// that canonical declaration.
> MergedDeclsMap MergedDecls;
>
> - typedef llvm::DenseMap<serialization::GlobalDeclID,
> - SmallVector<serialization::DeclID, 2> >
> - StoredMergedDeclsMap;
> -
> - /// \brief A mapping from canonical declaration IDs to the set of additional
> - /// declaration IDs that have been merged with that canonical declaration.
> - ///
> - /// This is the deserialized representation of the entries in MergedDecls.
> - /// When we query entries in MergedDecls, they will be augmented with entries
> - /// from StoredMergedDecls.
> - StoredMergedDeclsMap StoredMergedDecls;
> -
> - /// \brief Combine the stored merged declarations for the given canonical
> - /// declaration into the set of merged declarations.
> - ///
> - /// \returns An iterator into MergedDecls that corresponds to the position of
> - /// the given canonical declaration.
> - MergedDeclsMap::iterator
> - combineStoredMergedDecls(Decl *Canon, serialization::GlobalDeclID CanonID);
> -
> /// \brief A mapping from DeclContexts to the semantic DeclContext that we
> /// are treating as the definition of the entity. This is used, for instance,
> /// when merging implicit instantiations of class templates across modules.
> @@ -1188,7 +1168,7 @@ private:
> RecordLocation DeclCursorForID(serialization::DeclID ID,
> unsigned &RawLocation);
> void loadDeclUpdateRecords(serialization::DeclID ID, Decl *D);
> - void loadPendingDeclChain(serialization::GlobalDeclID ID);
> + void loadPendingDeclChain(Decl *D);
> void loadObjCCategories(serialization::GlobalDeclID ID, ObjCInterfaceDecl *D,
> unsigned PreviousGeneration = 0);
>
>
> Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Thu Mar 5 17:24:12 2015
> @@ -499,7 +499,6 @@ private:
> void WriteOpenCLExtensions(Sema &SemaRef);
> void WriteObjCCategories();
> void WriteRedeclarations();
> - void WriteMergedDecls();
> void WriteLateParsedTemplates(Sema &SemaRef);
> void WriteOptimizePragmaOptions(Sema &SemaRef);
>
>
> Modified: cfe/trunk/lib/AST/Decl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/Decl.cpp (original)
> +++ cfe/trunk/lib/AST/Decl.cpp Thu Mar 5 17:24:12 2015
> @@ -1489,6 +1489,11 @@ static bool isRedeclarable(Decl::Kind K)
> bool NamedDecl::declarationReplaces(NamedDecl *OldD, bool IsKnownNewer) const {
> assert(getDeclName() == OldD->getDeclName() && "Declaration name mismatch");
>
> + // Never replace one imported declaration with another; we need both results
> + // when re-exporting.
> + if (OldD->isFromASTFile() && isFromASTFile())
> + return false;
> +
> if (!isKindReplaceableBy(OldD->getKind(), getKind()))
> return false;
>
>
> Modified: cfe/trunk/lib/Sema/IdentifierResolver.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/IdentifierResolver.cpp?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/IdentifierResolver.cpp (original)
> +++ cfe/trunk/lib/Sema/IdentifierResolver.cpp Thu Mar 5 17:24:12 2015
> @@ -266,6 +266,11 @@ static DeclMatchKind compareDeclarations
>
> // If the declarations are redeclarations of each other, keep the newest one.
> if (Existing->getCanonicalDecl() == New->getCanonicalDecl()) {
> + // If we're adding an imported declaration, don't replace another imported
> + // declaration.
> + if (Existing->isFromASTFile() && New->isFromASTFile())
> + return DMK_Different;
> +
> // If either of these is the most recent declaration, use it.
> Decl *MostRecent = Existing->getMostRecentDecl();
> if (Existing == MostRecent)
>
> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Thu Mar 5 17:24:12 2015
> @@ -415,7 +415,9 @@ void LookupResult::resolveKind() {
> // If it's not unique, pull something off the back (and
> // continue at this index).
> // FIXME: This is wrong. We need to take the more recent declaration in
> - // order to get the right type, default arguments, etc.
> + // order to get the right type, default arguments, etc. We also need to
> + // prefer visible declarations to hidden ones (for redeclaration lookup
> + // in modules builds).
> Decls[I] = Decls[--N];
> continue;
> }
>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Mar 5 17:24:12 2015
> @@ -3316,16 +3316,6 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
> break;
> }
>
> - case MERGED_DECLARATIONS: {
> - for (unsigned Idx = 0; Idx < Record.size(); /* increment in loop */) {
> - GlobalDeclID CanonID = getGlobalDeclID(F, Record[Idx++]);
> - SmallVectorImpl<GlobalDeclID> &Decls = StoredMergedDecls[CanonID];
> - for (unsigned N = Record[Idx++]; N > 0; --N)
> - Decls.push_back(getGlobalDeclID(F, Record[Idx++]));
> - }
> - break;
> - }
> -
> case MACRO_OFFSET: {
> if (F.LocalNumMacros != 0) {
> Error("duplicate MACRO_OFFSET record in AST file");
> @@ -6227,6 +6217,10 @@ ASTReader::getGlobalDeclID(ModuleFile &F
>
> bool ASTReader::isDeclIDFromModule(serialization::GlobalDeclID ID,
> ModuleFile &M) const {
> + // Predefined decls aren't from any module.
> + if (ID < NUM_PREDEF_DECL_IDS)
> + return false;
> +
> GlobalDeclMapType::const_iterator I = GlobalDeclMap.find(ID);
> assert(I != GlobalDeclMap.end() && "Corrupted global declaration map");
> return &M == I->second;
> @@ -6259,39 +6253,51 @@ SourceLocation ASTReader::getSourceLocat
> return ReadSourceLocation(*Rec.F, RawLocation);
> }
>
> -Decl *ASTReader::GetExistingDecl(DeclID ID) {
> - if (ID < NUM_PREDEF_DECL_IDS) {
> - switch ((PredefinedDeclIDs)ID) {
> - case PREDEF_DECL_NULL_ID:
> - return nullptr;
> +static Decl *getPredefinedDecl(ASTContext &Context, PredefinedDeclIDs ID) {
> + switch (ID) {
> + case PREDEF_DECL_NULL_ID:
> + return nullptr;
>
> - case PREDEF_DECL_TRANSLATION_UNIT_ID:
> - return Context.getTranslationUnitDecl();
> + case PREDEF_DECL_TRANSLATION_UNIT_ID:
> + return Context.getTranslationUnitDecl();
>
> - case PREDEF_DECL_OBJC_ID_ID:
> - return Context.getObjCIdDecl();
> + case PREDEF_DECL_OBJC_ID_ID:
> + return Context.getObjCIdDecl();
>
> - case PREDEF_DECL_OBJC_SEL_ID:
> - return Context.getObjCSelDecl();
> + case PREDEF_DECL_OBJC_SEL_ID:
> + return Context.getObjCSelDecl();
>
> - case PREDEF_DECL_OBJC_CLASS_ID:
> - return Context.getObjCClassDecl();
> + case PREDEF_DECL_OBJC_CLASS_ID:
> + return Context.getObjCClassDecl();
>
> - case PREDEF_DECL_OBJC_PROTOCOL_ID:
> - return Context.getObjCProtocolDecl();
> + case PREDEF_DECL_OBJC_PROTOCOL_ID:
> + return Context.getObjCProtocolDecl();
>
> - case PREDEF_DECL_INT_128_ID:
> - return Context.getInt128Decl();
> + case PREDEF_DECL_INT_128_ID:
> + return Context.getInt128Decl();
>
> - case PREDEF_DECL_UNSIGNED_INT_128_ID:
> - return Context.getUInt128Decl();
> + case PREDEF_DECL_UNSIGNED_INT_128_ID:
> + return Context.getUInt128Decl();
>
> - case PREDEF_DECL_OBJC_INSTANCETYPE_ID:
> - return Context.getObjCInstanceTypeDecl();
> + case PREDEF_DECL_OBJC_INSTANCETYPE_ID:
> + return Context.getObjCInstanceTypeDecl();
> +
> + case PREDEF_DECL_BUILTIN_VA_LIST_ID:
> + return Context.getBuiltinVaListDecl();
> + }
> +}
>
> - case PREDEF_DECL_BUILTIN_VA_LIST_ID:
> - return Context.getBuiltinVaListDecl();
> +Decl *ASTReader::GetExistingDecl(DeclID ID) {
> + if (ID < NUM_PREDEF_DECL_IDS) {
> + Decl *D = getPredefinedDecl(Context, (PredefinedDeclIDs)ID);
> + if (D) {
> + // Track that we have merged the declaration with ID \p ID into the
> + // pre-existing predefined declaration \p D.
> + auto &Merged = MergedDecls[D->getCanonicalDecl()];
> + if (Merged.empty())
> + Merged.push_back(ID);
> }
> + return D;
> }
>
> unsigned Index = ID - NUM_PREDEF_DECL_IDS;
> @@ -8302,9 +8308,11 @@ void ASTReader::finishPendingActions() {
> PendingIncompleteDeclChains.clear();
>
> // Load pending declaration chains.
> - for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
> + for (unsigned I = 0; I != PendingDeclChains.size(); ++I) {
> loadPendingDeclChain(PendingDeclChains[I]);
> - PendingDeclChainsKnown.clear();
> + PendingDeclChainsKnown.erase(PendingDeclChains[I]);
> + }
> + assert(PendingDeclChainsKnown.empty());
> PendingDeclChains.clear();
>
> // Make the most recent of the top-level declarations visible.
>
> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Thu Mar 5 17:24:12 2015
> @@ -123,9 +123,6 @@ namespace clang {
> /// \brief RAII class used to capture the first ID within a redeclaration
> /// chain and to introduce it into the list of pending redeclaration chains
> /// on destruction.
> - ///
> - /// The caller can choose not to introduce this ID into the list of pending
> - /// redeclaration chains by calling \c suppress().
> class RedeclarableResult {
> ASTReader &Reader;
> GlobalDeclID FirstID;
> @@ -149,9 +146,11 @@ namespace clang {
> }
>
> ~RedeclarableResult() {
> - if (FirstID && Owning && isRedeclarableDeclKind(DeclKind) &&
> - Reader.PendingDeclChainsKnown.insert(FirstID).second)
> - Reader.PendingDeclChains.push_back(FirstID);
> + if (FirstID && Owning && isRedeclarableDeclKind(DeclKind)) {
> + auto Canon = Reader.GetDecl(FirstID)->getCanonicalDecl();
> + if (Reader.PendingDeclChainsKnown.insert(Canon).second)
> + Reader.PendingDeclChains.push_back(Canon);
> + }
> }
>
> /// \brief Retrieve the first ID.
> @@ -160,12 +159,6 @@ namespace clang {
> /// \brief Get a known declaration that this should be merged with, if
> /// any.
> Decl *getKnownMergeTarget() const { return MergeWith; }
> -
> - /// \brief Do not introduce this declaration ID into the set of pending
> - /// declaration chains.
> - void suppress() {
> - Owning = false;
> - }
> };
>
> /// \brief Class used to capture the result of searching for an existing
> @@ -2076,12 +2069,14 @@ ASTDeclReader::VisitRedeclarable(Redecla
> // and is used for space optimization.
> if (FirstDeclID == 0)
> FirstDeclID = ThisDeclID;
> - else if (Record[Idx++]) {
> - // We need to merge with FirstDeclID. Read it now to ensure that it is
> - // before us in the redecl chain, then forget we saw it so that we will
> - // merge with it.
> - MergeWith = Reader.GetDecl(FirstDeclID);
> - FirstDeclID = ThisDeclID;
> + else if (unsigned N = Record[Idx++]) {
> + // We have some declarations that must be before us in our redeclaration
> + // chain. Read them now, and remember that we ought to merge with one of
> + // them.
> + // FIXME: Provide a known merge target to the second and subsequent such
> + // declaration.
> + for (unsigned I = 0; I != N; ++I)
> + MergeWith = ReadDecl(Record, Idx/*, MergeWith*/);
> }
>
> T *FirstDecl = cast_or_null<T>(Reader.GetDecl(FirstDeclID));
> @@ -2109,17 +2104,6 @@ void ASTDeclReader::mergeRedeclarable(Re
> RedeclarableResult &Redecl,
> DeclID TemplatePatternID) {
> T *D = static_cast<T*>(DBase);
> - T *DCanon = D->getCanonicalDecl();
> - if (D != DCanon &&
> - // IDs < NUM_PREDEF_DECL_IDS are not loaded from an AST file.
> - Redecl.getFirstID() >= NUM_PREDEF_DECL_IDS &&
> - (!Reader.getContext().getLangOpts().Modules ||
> - Reader.getOwningModuleFile(DCanon) == Reader.getOwningModuleFile(D))) {
> - // All redeclarations between this declaration and its originally-canonical
> - // declaration get pulled in when we load DCanon; we don't need to
> - // perform any more merging now.
> - Redecl.suppress();
> - }
>
> // If modules are not available, there is no reason to perform this merge.
> if (!Reader.getContext().getLangOpts().Modules)
> @@ -2215,10 +2199,9 @@ void ASTDeclReader::mergeRedeclarable(Re
> // that. We accept the linear algorithm here because the number of
> // unique canonical declarations of an entity should always be tiny.
> if (DCanon == D) {
> - SmallVectorImpl<DeclID> &Merged = Reader.MergedDecls[ExistingCanon];
> - if (std::find(Merged.begin(), Merged.end(), Redecl.getFirstID())
> - == Merged.end())
> - Merged.push_back(Redecl.getFirstID());
> + Reader.MergedDecls[ExistingCanon].push_back(Redecl.getFirstID());
> + if (Reader.PendingDeclChainsKnown.insert(ExistingCanon).second)
> + Reader.PendingDeclChains.push_back(ExistingCanon);
> }
> }
> }
> @@ -2697,8 +2680,6 @@ ASTDeclReader::FindExistingResult ASTDec
> // unmergeable contexts.
> FindExistingResult Result(Reader, D, /*Existing=*/nullptr,
> AnonymousDeclNumber, TypedefNameForLinkage);
> - // FIXME: We may still need to pull in the redeclaration chain; there can
> - // be redeclarations via 'decltype'.
> Result.suppress();
> return Result;
> }
> @@ -2930,29 +2911,6 @@ void ASTReader::markIncompleteDeclChain(
> }
> }
>
> -ASTReader::MergedDeclsMap::iterator
> -ASTReader::combineStoredMergedDecls(Decl *Canon, GlobalDeclID CanonID) {
> - // If we don't have any stored merged declarations, just look in the
> - // merged declarations set.
> - StoredMergedDeclsMap::iterator StoredPos = StoredMergedDecls.find(CanonID);
> - if (StoredPos == StoredMergedDecls.end())
> - return MergedDecls.find(Canon);
> -
> - // Append the stored merged declarations to the merged declarations set.
> - MergedDeclsMap::iterator Pos = MergedDecls.find(Canon);
> - if (Pos == MergedDecls.end())
> - Pos = MergedDecls.insert(std::make_pair(Canon,
> - SmallVector<DeclID, 2>())).first;
> - Pos->second.append(StoredPos->second.begin(), StoredPos->second.end());
> - StoredMergedDecls.erase(StoredPos);
> -
> - // Sort and uniquify the set of merged declarations.
> - llvm::array_pod_sort(Pos->second.begin(), Pos->second.end());
> - Pos->second.erase(std::unique(Pos->second.begin(), Pos->second.end()),
> - Pos->second.end());
> - return Pos;
> -}
> -
> /// \brief Read the declaration at the given offset from the AST file.
> Decl *ASTReader::ReadDeclRecord(DeclID ID) {
> unsigned Index = ID - NUM_PREDEF_DECL_IDS;
> @@ -3362,25 +3320,25 @@ namespace {
> };
> }
>
> -void ASTReader::loadPendingDeclChain(serialization::GlobalDeclID ID) {
> - Decl *D = GetDecl(ID);
> - Decl *CanonDecl = D->getCanonicalDecl();
> -
> +void ASTReader::loadPendingDeclChain(Decl *CanonDecl) {
> + // The decl might have been merged into something else after being added to
> + // our list. If it was, just skip it.
> + if (!CanonDecl->isCanonicalDecl())
> + return;
> +
> // Determine the set of declaration IDs we'll be searching for.
> - SmallVector<DeclID, 1> SearchDecls;
> - GlobalDeclID CanonID = 0;
> - if (D == CanonDecl) {
> - SearchDecls.push_back(ID); // Always first.
> - CanonID = ID;
> - }
> - MergedDeclsMap::iterator MergedPos = combineStoredMergedDecls(CanonDecl, ID);
> + SmallVector<DeclID, 16> SearchDecls;
> + GlobalDeclID CanonID = CanonDecl->getGlobalID();
> + if (CanonID)
> + SearchDecls.push_back(CanonDecl->getGlobalID()); // Always first.
> + MergedDeclsMap::iterator MergedPos = MergedDecls.find(CanonDecl);
> if (MergedPos != MergedDecls.end())
> SearchDecls.append(MergedPos->second.begin(), MergedPos->second.end());
> -
> +
> // Build up the list of redeclarations.
> RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized, CanonID);
> ModuleMgr.visitDepthFirst(&RedeclChainVisitor::visit, &Visitor);
> -
> +
> // Retrieve the chains.
> ArrayRef<Decl *> Chain = Visitor.getChain();
> if (Chain.empty())
>
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Mar 5 17:24:12 2015
> @@ -924,7 +924,6 @@ void ASTWriter::WriteBlockInfoBlock() {
> RECORD(OBJC_CATEGORIES_MAP);
> RECORD(FILE_SORTED_DECLS);
> RECORD(IMPORTED_MODULES);
> - RECORD(MERGED_DECLARATIONS);
> RECORD(LOCAL_REDECLARATIONS);
> RECORD(OBJC_CATEGORIES);
> RECORD(MACRO_OFFSET);
> @@ -3115,7 +3114,7 @@ static NamedDecl *getDeclForLocalLookup(
> for (; Redecl; Redecl = Redecl->getPreviousDecl()) {
> if (!Redecl->isFromASTFile())
> return cast<NamedDecl>(Redecl);
> - // If we come up a decl from a (chained-)PCH stop since we won't find a
> + // If we find a decl from a (chained-)PCH stop since we won't find a
> // local one.
> if (D->getOwningModuleID() == 0)
> break;
> @@ -3671,14 +3670,24 @@ void ASTWriter::visitLocalLookupResults(
>
> SmallVector<DeclarationName, 16> ExternalNames;
> for (auto &Lookup : *DC->buildLookup()) {
> - // If there are no local declarations in our lookup result, we don't
> - // need to write an entry for the name at all unless we're rewriting
> - // the decl context.
> - if (!Lookup.second.hasLocalDecls() && !isRewritten(cast<Decl>(DC)))
> - continue;
> -
> if (Lookup.second.hasExternalDecls() ||
> DC->NeedToReconcileExternalVisibleStorage) {
> + // If there are no local declarations in our lookup result, we don't
> + // need to write an entry for the name at all unless we're rewriting
> + // the decl context. If we can't write out a lookup set without
> + // performing more deserialization, just skip this entry.
> + if (!isRewritten(cast<Decl>(DC))) {
> + bool AllFromASTFile = true;
> + for (auto *D : Lookup.second.getLookupResult()) {
> + AllFromASTFile &=
> + getDeclForLocalLookup(getLangOpts(), D)->isFromASTFile();
> + if (!AllFromASTFile)
> + break;
> + }
> + if (AllFromASTFile)
> + continue;
> + }
> +
> // We don't know for sure what declarations are found by this name,
> // because the external source might have a different set from the set
> // that are in the lookup map, and we can't update it now without
> @@ -3898,10 +3907,6 @@ void ASTWriter::WriteRedeclarations() {
> if (Prev->isFromASTFile())
> FirstFromAST = Prev;
> }
> -
> - // FIXME: Do we need to do this for the first declaration from each
> - // redeclaration chain that was merged into this one?
> - Chain->MergedDecls[FirstFromAST].push_back(getDeclID(First));
> }
>
> LocalRedeclChains[Offset] = Size;
> @@ -3998,25 +4003,6 @@ void ASTWriter::WriteObjCCategories() {
> Stream.EmitRecord(OBJC_CATEGORIES, Categories);
> }
>
> -void ASTWriter::WriteMergedDecls() {
> - if (!Chain || Chain->MergedDecls.empty())
> - return;
> -
> - RecordData Record;
> - for (ASTReader::MergedDeclsMap::iterator I = Chain->MergedDecls.begin(),
> - IEnd = Chain->MergedDecls.end();
> - I != IEnd; ++I) {
> - DeclID CanonID = I->first->isFromASTFile()? I->first->getGlobalID()
> - : GetDeclRef(I->first);
> - assert(CanonID && "Merged declaration not known?");
> -
> - Record.push_back(CanonID);
> - Record.push_back(I->second.size());
> - Record.append(I->second.begin(), I->second.end());
> - }
> - Stream.EmitRecord(MERGED_DECLARATIONS, Record);
> -}
> -
> void ASTWriter::WriteLateParsedTemplates(Sema &SemaRef) {
> Sema::LateParsedTemplateMapT &LPTMap = SemaRef.LateParsedTemplateMap;
>
> @@ -4699,7 +4685,6 @@ void ASTWriter::WriteASTCore(Sema &SemaR
>
> WriteDeclReplacementsBlock();
> WriteRedeclarations();
> - WriteMergedDecls();
> WriteObjCCategories();
> WriteLateParsedTemplates(SemaRef);
> if(!WritingModule)
>
> Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Thu Mar 5 17:24:12 2015
> @@ -1461,24 +1461,41 @@ void ASTDeclWriter::VisitRedeclarable(Re
> assert(isRedeclarableDeclKind(static_cast<T *>(D)->getKind()) &&
> "Not considered redeclarable?");
>
> + // There is more than one declaration of this entity, so we will need to
> + // write a redeclaration chain.
> + Writer.AddDeclRef(First, Record);
> + Writer.Redeclarations.insert(First);
> +
> auto *Previous = D->getPreviousDecl();
> - auto *FirstToEmit = First;
> - if (Context.getLangOpts().Modules && Writer.Chain && !Previous) {
> - // In a modules build, we can have imported declarations after a local
> - // canonical declaration. If we do, we want to treat the first imported
> - // declaration as our canonical declaration on reload, in order to
> - // rebuild the redecl chain in the right order.
> +
> + // In a modules build, we can have imported declarations after a local
> + // canonical declaration. If this is the first local declaration, emit
> + // a list of all such imported declarations so that we can ensure they
> + // are loaded before we are. This allows us to rebuild the redecl chain
> + // in the right order on reload (all declarations imported by a module
> + // should be before all declarations provided by that module).
> + bool EmitImportedMergedCanonicalDecls = false;
> + if (Context.getLangOpts().Modules && Writer.Chain) {
> + auto *PreviousLocal = Previous;
> + while (PreviousLocal && PreviousLocal->isFromASTFile())
> + PreviousLocal = PreviousLocal->getPreviousDecl();
> + if (!PreviousLocal)
> + EmitImportedMergedCanonicalDecls = true;
> + }
> + if (EmitImportedMergedCanonicalDecls) {
> + llvm::SmallMapVector<ModuleFile*, Decl*, 16> FirstInModule;
> for (auto *Redecl = MostRecent; Redecl;
> Redecl = Redecl->getPreviousDecl())
> if (Redecl->isFromASTFile())
> - FirstToEmit = Redecl;
> - }
> -
> - // There is more than one declaration of this entity, so we will need to
> - // write a redeclaration chain.
> - Writer.AddDeclRef(FirstToEmit, Record);
> - Record.push_back(FirstToEmit != First);
> - Writer.Redeclarations.insert(First);
> + FirstInModule[Writer.Chain->getOwningModuleFile(Redecl)] = Redecl;
> + // FIXME: If FirstInModule has entries for modules A and B, and B imports
> + // A (directly or indirectly), we don't need to write the entry for A.
> + Record.push_back(FirstInModule.size());
> + for (auto I = FirstInModule.rbegin(), E = FirstInModule.rend();
> + I != E; ++I)
> + Writer.AddDeclRef(I->second, Record);
> + } else
> + Record.push_back(0);
>
> // Make sure that we serialize both the previous and the most-recent
> // declarations, which (transitively) ensures that all declarations in the
>
> Modified: cfe/trunk/test/Modules/linkage-merge.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/linkage-merge.cpp?rev=231424&r1=231423&r2=231424&view=diff
> ==============================================================================
> --- cfe/trunk/test/Modules/linkage-merge.cpp (original)
> +++ cfe/trunk/test/Modules/linkage-merge.cpp Thu Mar 5 17:24:12 2015
> @@ -7,5 +7,10 @@ static int f(int);
> int f(int);
>
> static void g(int);
> -// expected-error at -1 {{functions that differ only in their return type cannot be overloaded}}
> -// expected-note at Inputs/linkage-merge-foo.h:2 {{previous declaration is here}}
> +// FIXME: Whether we notice the problem here depends on the order in which we
> +// happen to find lookup results for 'g'; LookupResult::resolveKind needs to
> +// be taught to prefer a visible result over a non-visible one.
> +//
> +// FIXME-error at -1 {{functions that differ only in their return type cannot be overloaded}}
> +// FIXME-note at Inputs/linkage-merge-foo.h:2 {{previous declaration is here}}
> +// expected-no-diagnostics
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list