r231424 - [modules] Rework merging of redeclaration chains on module import.
Argyrios Kyrtzidis
kyrtzidis at apple.com
Wed Mar 25 08:09:59 PDT 2015
> On Mar 20, 2015, at 3:00 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Mon, Mar 9, 2015 at 11:13 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com <mailto:kyrtzidis at apple.com>> wrote:
>
>> On Mar 9, 2015, at 5:20 PM, Richard Smith <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>> wrote:
>>
>> On Sat, Mar 7, 2015 at 8:21 AM, Argyrios Kyrtzidis <kyrtzidis at apple.com <mailto:kyrtzidis at apple.com>> 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 <richard-llvm at metafoo.co.uk <mailto: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 <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 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 <mailto:cfe-commits at cs.uiuc.edu>
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150325/0e9413a3/attachment.html>
More information about the cfe-commits
mailing list