r231424 - [modules] Rework merging of redeclaration chains on module import.
Richard Smith
richard at metafoo.co.uk
Mon Mar 9 17:20:21 PDT 2015
On Sat, Mar 7, 2015 at 8:21 AM, Argyrios Kyrtzidis <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.
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>
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150309/e210f1d7/attachment.html>
More information about the cfe-commits
mailing list