> On Mar 20, 2015, at 3:00 PM, Richard Smith <[email protected]> wrote: > > On Mon, Mar 9, 2015 at 11:13 PM, Argyrios Kyrtzidis <[email protected] > <mailto:[email protected]>> wrote: > >> On Mar 9, 2015, at 5:20 PM, Richard Smith <[email protected] >> <mailto:[email protected]>> wrote: >> >> On Sat, Mar 7, 2015 at 8:21 AM, Argyrios Kyrtzidis <[email protected] >> <mailto:[email protected]>> wrote: >> 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 ? >> >> It depends. If these declarations are all declared in the same module, then >> no, this is not expected; the lookup table should be unchanged in that case, >> and should only contain the most recent declaration. That'd be a bug; if >> this is the case, please send me a testcase and I'll dig into it. > > Attached a few files, try this: > > - $ cd mod-test > - Debug this invocation: > $ env CINDEXTEST_COMPLETION_CACHING=1 c-index-test > -code-completion-at=t.m:2:1 t.m -fmodules -I . > > In CodeCompletionDeclConsumer::FoundDecl() (at SemaCodeComplete.cpp), watch > which declarations are passed to this function, you’ll notice that all > re-declarations of the same module (as in my original example) are getting > through here. > > Ah, I see. The problem is that we don't usually build a DeclContext lookup > table in C, and thus there is no such table in the module file for us to use. > This was broken even before my change (LookupVisibleDecls assumed that > looking in the DeclContext table is an appropriate thing to do even for the > TU in C, which doesn't work if there's an external source). > > I've switched LookupVisibleDecls to use IdentifierInfo lookup when looking up > into the TU outside C++, and made a couple of other fixes for this. Please > let me know if you see more issues.
Seems to work fine now, thank you! >> If they come from sibling modules, then yes, it's expected that the lookup >> tables would now contain all the most recent declarations from the >> directly-imported modules -- perhaps LookupVisibleDecl should be keeping >> only one declaration of each entity, if that's what it wants (the other >> consumers of decl context lookup results do this, and this is not the only >> way in which you can get multiple NamedDecls back from a lookup which are >> redeclarations of the same entity). That's either not a bug or a >> pre-existing bug, depending on how we think LookupVisibleDecl should behave. >> >> > On Mar 5, 2015, at 3:24 PM, Richard Smith <[email protected] >> > <mailto:[email protected]>> 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 >> > <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 >> > >> > <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 >> > >> > <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 >> > >> > <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 >> > >> > <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 >> > >> > <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 >> > >> > <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 >> > >> > <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 >> > >> > <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 >> > >> > <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 >> > >> > <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 >> > >> > <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 >> > >> > <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@-1 {{functions that differ only in their return type >> > cannot be overloaded}} >> > -// expected-note@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@-1 {{functions that differ only in their return type >> > cannot be overloaded}} >> > +// FIXME-note@Inputs/linkage-merge-foo.h:2 {{previous declaration is >> > here}} >> > +// expected-no-diagnostics >> > >> > >> > _______________________________________________ >> > cfe-commits mailing list >> > [email protected] <mailto:[email protected]> >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits> >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] <mailto:[email protected]> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits> > > > _______________________________________________ > cfe-commits mailing list > [email protected] <mailto:[email protected]> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
