r245779 - [modules] Rearrange how redeclaration chains are loaded, to remove a walk over

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 27 14:39:39 PDT 2015


On Thu, Aug 27, 2015 at 10:47 AM, Justin Bogner <mail at justinbogner.com>
wrote:

> Richard Smith via cfe-commits <cfe-commits at lists.llvm.org> writes:
> > Author: rsmith
> > Date: Fri Aug 21 20:47:18 2015
> > New Revision: 245779
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=245779&view=rev
> > Log:
> > [modules] Rearrange how redeclaration chains are loaded, to remove a
> walk over
> > all modules and reduce the number of declarations we load when loading a
> > redeclaration chain.
>
> It looks like ASTDeclWriter::AddFirstDeclFromEachModule is called with a
> null `this` since this change, which has been causing ubsan failures on
> 76 tests since it went in:
>
>   ASTWriterDecl.cpp:170:18: runtime error: member call on null pointer of
> type 'clang::ASTReader'
>
> Logs and other failures here:
>
>   http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/146/
>
> I guess you must've missed the failure email. Could you please take a
> look?


Thanks, fixed in r246215.


> > The new approach is:
> >  * when loading the first declaration of an entity within a module file,
> we
> >    first load all declarations of the entity that were imported into that
> >    module file, and then load all the other declarations of that entity
> from
> >    that module file and build a suitable decl chain from them
> >  * when loading any other declaration of an entity, we first load the
> first
> >    declaration from the same module file
> >
> > As before, we complete redecl chains through name lookup where necessary.
> >
> > To make this work, I also had to change the way that template
> specializations
> > are stored -- it no longer suffices to track only canonical
> specializations; we
> > now emit all "first local" declarations when emitting a list of
> specializations
> > for a template.
> >
> > On one testcase with several thousand imported module files, this
> reduces the
> > total runtime by 72%.
> >
> > Modified:
> >     cfe/trunk/include/clang/Serialization/ASTWriter.h
> >     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/cxx-templates.cpp
> >
> > Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=245779&r1=245778&r2=245779&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
> > +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Fri Aug 21
> 20:47:18 2015
> > @@ -405,6 +405,10 @@ private:
> >    /// \brief The set of declarations that may have redeclaration chains
> that
> >    /// need to be serialized.
> >    llvm::SmallVector<const Decl *, 16> Redeclarations;
> > +
> > +  /// \brief A cache of the first local declaration for "interesting"
> > +  /// redeclaration chains.
> > +  llvm::DenseMap<const Decl *, const Decl *> FirstLocalDeclCache;
> >
> >    /// \brief Statements that we've encountered while serializing a
> >    /// declaration or type.
> > @@ -676,6 +680,10 @@ public:
> >                            const ASTTemplateArgumentListInfo
> *ASTTemplArgList,
> >                            RecordDataImpl &Record);
> >
> > +  /// \brief Find the first local declaration of a given local
> redeclarable
> > +  /// decl.
> > +  const Decl *getFirstLocalDecl(const Decl *D);
> > +
> >    /// \brief Emit a reference to a declaration.
> >    void AddDeclRef(const Decl *D, RecordDataImpl &Record);
> >
> > @@ -857,12 +865,6 @@ public:
> >    void CompletedTagDefinition(const TagDecl *D) override;
> >    void AddedVisibleDecl(const DeclContext *DC, const Decl *D) override;
> >    void AddedCXXImplicitMember(const CXXRecordDecl *RD, const Decl *D)
> override;
> > -  void AddedCXXTemplateSpecialization(const ClassTemplateDecl *TD,
> > -                             const ClassTemplateSpecializationDecl *D)
> override;
> > -  void AddedCXXTemplateSpecialization(const VarTemplateDecl *TD,
> > -                               const VarTemplateSpecializationDecl *D)
> override;
> > -  void AddedCXXTemplateSpecialization(const FunctionTemplateDecl *TD,
> > -                                      const FunctionDecl *D) override;
> >    void ResolvedExceptionSpec(const FunctionDecl *FD) override;
> >    void DeducedReturnType(const FunctionDecl *FD, QualType ReturnType)
> override;
> >    void ResolvedOperatorDelete(const CXXDestructorDecl *DD,
> >
> > Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=245779&r1=245778&r2=245779&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> > +++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Aug 21 20:47:18 2015
> > @@ -8123,11 +8123,8 @@ void ASTReader::finishPendingActions() {
> >      PendingIncompleteDeclChains.clear();
> >
> >      // Load pending declaration chains.
> > -    for (unsigned I = 0; I != PendingDeclChains.size(); ++I) {
> > -      PendingDeclChainsKnown.erase(PendingDeclChains[I]);
> > +    for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
> >        loadPendingDeclChain(PendingDeclChains[I]);
> > -    }
> > -    assert(PendingDeclChainsKnown.empty());
> >      PendingDeclChains.clear();
> >
> >      assert(RedeclsDeserialized.empty() && "some redecls not wired up");
> >
> > Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=245779&r1=245778&r2=245779&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
> > +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Aug 21 20:47:18
> 2015
> > @@ -147,12 +147,6 @@ namespace clang {
> >        }
> >
> >        ~RedeclarableResult() {
> > -        if (FirstID && Owning &&
> > -            isRedeclarableDeclKind(LoadedDecl->getKind())) {
> > -          auto Canon = Reader.GetDecl(FirstID)->getCanonicalDecl();
> > -          if (Reader.PendingDeclChainsKnown.insert(Canon).second)
> > -            Reader.PendingDeclChains.push_back(Canon);
> > -        }
> >        }
> >
> >        /// \brief Note that a RedeclarableDecl is not actually
> redeclarable.
> > @@ -2186,23 +2180,33 @@ ASTDeclReader::RedeclarableResult
> >  ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {
> >    DeclID FirstDeclID = ReadDeclID(Record, Idx);
> >    Decl *MergeWith = nullptr;
> > +
> >    bool IsKeyDecl = ThisDeclID == FirstDeclID;
> > +  bool IsFirstLocalDecl = false;
> >
> >    // 0 indicates that this declaration was the only declaration of its
> entity,
> >    // and is used for space optimization.
> >    if (FirstDeclID == 0) {
> >      FirstDeclID = ThisDeclID;
> >      IsKeyDecl = true;
> > +    IsFirstLocalDecl = true;
> >    } else if (unsigned N = Record[Idx++]) {
> > -    IsKeyDecl = false;
> > +    // This declaration was the first local declaration, but may have
> imported
> > +    // other declarations.
> > +    IsKeyDecl = N == 1;
> > +    IsFirstLocalDecl = true;
> >
> >      // 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)
> > +    for (unsigned I = 0; I != N - 1; ++I)
> >        MergeWith = ReadDecl(Record, Idx/*, MergeWith*/);
> > +  } else {
> > +    // This declaration was not the first local declaration. Read the
> first
> > +    // local declaration now, to trigger the import of other
> redeclarations.
> > +    (void)ReadDecl(Record, Idx);
> >    }
> >
> >    T *FirstDecl = cast_or_null<T>(Reader.GetDecl(FirstDeclID));
> > @@ -2214,13 +2218,21 @@ ASTDeclReader::VisitRedeclarable(Redecla
> >      D->RedeclLink = Redeclarable<T>::PreviousDeclLink(FirstDecl);
> >      D->First = FirstDecl->getCanonicalDecl();
> >    }
> > -
> > +
> >    // Note that this declaration has been deserialized.
> > -  Reader.RedeclsDeserialized.insert(static_cast<T *>(D));
> > -
> > +  T *DAsT = static_cast<T*>(D);
> > +  Reader.RedeclsDeserialized.insert(DAsT);
> > +
> > +  // Note that we need to load local redeclarations of this decl and
> build a
> > +  // decl chain for them. This must happen *after* we perform the
> preloading
> > +  // above; this ensures that the redeclaration chain is built in the
> correct
> > +  // order.
> > +  if (IsFirstLocalDecl)
> > +    Reader.PendingDeclChains.push_back(DAsT);
> > +
> >    // The result structure takes care to note that we need to load the
> >    // other declaration chains for this ID.
> > -  return RedeclarableResult(Reader, FirstDeclID, static_cast<T *>(D),
> MergeWith,
> > +  return RedeclarableResult(Reader, FirstDeclID, DAsT, MergeWith,
> >                              IsKeyDecl);
> >  }
> >
> > @@ -2330,11 +2342,8 @@ void ASTDeclReader::mergeRedeclarable(Re
> >            TemplatePatternID, Redecl.isKeyDecl());
> >
> >      // If this declaration is a key declaration, make a note of that.
> > -    if (Redecl.isKeyDecl()) {
> > +    if (Redecl.isKeyDecl())
> >        Reader.KeyDecls[ExistingCanon].push_back(Redecl.getFirstID());
> > -      if (Reader.PendingDeclChainsKnown.insert(ExistingCanon).second)
> > -        Reader.PendingDeclChains.push_back(ExistingCanon);
> > -    }
> >    }
> >  }
> >
> > @@ -3424,15 +3433,12 @@ namespace {
> >      ASTReader &Reader;
> >      SmallVectorImpl<DeclID> &SearchDecls;
> >      llvm::SmallPtrSetImpl<Decl *> &Deserialized;
> > -    GlobalDeclID CanonID;
> >      SmallVector<Decl *, 4> Chain;
> >
> >    public:
> >      RedeclChainVisitor(ASTReader &Reader, SmallVectorImpl<DeclID>
> &SearchDecls,
> > -                       llvm::SmallPtrSetImpl<Decl *> &Deserialized,
> > -                       GlobalDeclID CanonID)
> > -      : Reader(Reader), SearchDecls(SearchDecls),
> Deserialized(Deserialized),
> > -        CanonID(CanonID) {
> > +                       llvm::SmallPtrSetImpl<Decl *> &Deserialized)
> > +      : Reader(Reader), SearchDecls(SearchDecls),
> Deserialized(Deserialized) {
> >        assert(std::is_sorted(SearchDecls.begin(), SearchDecls.end()));
> >      }
> >
> > @@ -3518,10 +3524,10 @@ namespace {
> >            break;
> >        }
> >
> > -      if (LocalSearchDeclID && LocalSearchDeclID != CanonID) {
> > +      assert(LocalSearchDeclID);
> > +      if (LocalSearchDeclID) {
> >          // If the search decl was from this module, add it to the chain.
> >          // Note, the chain is sorted from newest to oldest, so this
> goes last.
> > -        // We exclude the canonical declaration; it implicitly goes at
> the end.
> >          addToChain(Reader.GetDecl(LocalSearchDeclID));
> >        }
> >
> > @@ -3531,26 +3537,14 @@ namespace {
> >    };
> >  }
> >
> > -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;
> > -
> > +void ASTReader::loadPendingDeclChain(Decl *FirstLocal) {
> >    // Determine the set of declaration IDs we'll be searching for.
> > -  SmallVector<DeclID, 16> SearchDecls;
> > -  GlobalDeclID CanonID = CanonDecl->getGlobalID();
> > -  if (CanonID)
> > -    SearchDecls.push_back(CanonDecl->getGlobalID()); // Always first.
> > -  KeyDeclsMap::iterator KeyPos = KeyDecls.find(CanonDecl);
> > -  if (KeyPos != KeyDecls.end())
> > -    SearchDecls.append(KeyPos->second.begin(), KeyPos->second.end());
> > -  llvm::array_pod_sort(SearchDecls.begin(), SearchDecls.end());
> > +  SmallVector<DeclID, 1> SearchDecls;
> > +  SearchDecls.push_back(FirstLocal->getGlobalID());
> >
> >    // Build up the list of redeclarations.
> > -  RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized,
> CanonID);
> > -  ModuleMgr.visit(Visitor);
> > -  RedeclsDeserialized.erase(CanonDecl);
> > +  RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized);
> > +  Visitor(*getOwningModuleFile(FirstLocal));
> >
> >    // Retrieve the chains.
> >    ArrayRef<Decl *> Chain = Visitor.getChain();
> > @@ -3561,11 +3555,14 @@ void ASTReader::loadPendingDeclChain(Dec
> >    //
> >    // FIXME: We have three different dispatches on decl kind here; maybe
> >    // we should instead generate one loop per kind and dispatch up-front?
> > +  Decl *CanonDecl = FirstLocal->getCanonicalDecl();
> >    Decl *MostRecent = ASTDeclReader::getMostRecentDecl(CanonDecl);
> >    if (!MostRecent)
> >      MostRecent = CanonDecl;
> >    for (unsigned I = 0, N = Chain.size(); I != N; ++I) {
> >      auto *D = Chain[N - I - 1];
> > +    if (D == CanonDecl)
> > +      continue;
> >      ASTDeclReader::attachPreviousDecl(*this, D, MostRecent, CanonDecl);
> >      MostRecent = D;
> >    }
> >
> > Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=245779&r1=245778&r2=245779&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
> > +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Aug 21 20:47:18 2015
> > @@ -3799,41 +3799,39 @@ void ASTWriter::WriteRedeclarations() {
> >    SmallVector<serialization::LocalRedeclarationsInfo, 2>
> LocalRedeclsMap;
> >
> >    for (unsigned I = 0, N = Redeclarations.size(); I != N; ++I) {
> > -    const Decl *Key = Redeclarations[I];
> > -    assert((Chain ? Chain->getKeyDeclaration(Key) == Key
> > -                  : Key->isFirstDecl()) &&
> > -           "not the key declaration");
> > +    const Decl *FirstLocal = Redeclarations[I];
> > +    assert(!FirstLocal->isFromASTFile() &&
> > +           (!FirstLocal->getPreviousDecl() ||
> > +            FirstLocal->getPreviousDecl()->isFromASTFile() ||
> > +            getDeclID(FirstLocal->getPreviousDecl()) <
> NUM_PREDEF_DECL_IDS) &&
> > +           "not the first local declaration");
> > +    assert(getDeclID(FirstLocal) >= NUM_PREDEF_DECL_IDS &&
> > +           "should not have predefined decl as first decl");
> >
> > -    const Decl *First = Key->getCanonicalDecl();
> > -    const Decl *MostRecent = First->getMostRecentDecl();
> > -
> > -    assert((getDeclID(First) >= NUM_PREDEF_DECL_IDS || First == Key) &&
> > -           "should not have imported key decls for predefined decl");
> > -
> > -    // If we only have a single declaration, there is no point in
> storing
> > -    // a redeclaration chain.
> > -    if (First == MostRecent)
> > -      continue;
> > -
> >      unsigned Offset = LocalRedeclChains.size();
> >      unsigned Size = 0;
> >      LocalRedeclChains.push_back(0); // Placeholder for the size.
> >
> >      // Collect the set of local redeclarations of this declaration,
> from newest
> >      // to oldest.
> > -    for (const Decl *Prev = MostRecent; Prev;
> > -         Prev = Prev->getPreviousDecl()) {
> > -      if (!Prev->isFromASTFile() && Prev != Key) {
> > +    for (const Decl *Prev = FirstLocal->getMostRecentDecl(); Prev !=
> FirstLocal;
> > +         Prev = Prev->getPreviousDecl()) {
> > +      if (!Prev->isFromASTFile()) {
> >          AddDeclRef(Prev, LocalRedeclChains);
> >          ++Size;
> >        }
> >      }
> >
> > +    // If we only have a single local declaration, there is no point in
> storing
> > +    // a redeclaration chain.
> > +    if (LocalRedeclChains.size() == 1)
> > +      continue;
> > +
> >      LocalRedeclChains[Offset] = Size;
> >
> >      // Add the mapping from the first ID from the AST to the set of
> local
> >      // declarations.
> > -    LocalRedeclarationsInfo Info = { getDeclID(Key), Offset };
> > +    LocalRedeclarationsInfo Info = { getDeclID(FirstLocal), Offset };
> >      LocalRedeclsMap.push_back(Info);
> >
> >      assert(N == Redeclarations.size() &&
> > @@ -4145,8 +4143,6 @@ void ASTWriter::WriteASTCore(Sema &SemaR
> >      if (D) {
> >        assert(D->isCanonicalDecl() && "predefined decl is not
> canonical");
> >        DeclIDs[D] = ID;
> > -      if (D->getMostRecentDecl() != D)
> > -        Redeclarations.push_back(D);
> >      }
> >    };
> >    RegisterPredefDecl(Context.getTranslationUnitDecl(),
> > @@ -5730,42 +5726,6 @@ void ASTWriter::AddedCXXImplicitMember(c
> >    DeclUpdates[RD].push_back(DeclUpdate(UPD_CXX_ADDED_IMPLICIT_MEMBER,
> D));
> >  }
> >
> > -void ASTWriter::AddedCXXTemplateSpecialization(const ClassTemplateDecl
> *TD,
> > -                                     const
> ClassTemplateSpecializationDecl *D) {
> > -  // The specializations set is kept in the canonical template.
> > -  TD = TD->getCanonicalDecl();
> > -  if (!(!D->isFromASTFile() && TD->isFromASTFile()))
> > -    return; // Not a source specialization added to a template from PCH.
> > -
> > -  assert(!WritingAST && "Already writing the AST!");
> > -
> DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,
> > -                                       D));
> > -}
> > -
> > -void ASTWriter::AddedCXXTemplateSpecialization(
> > -    const VarTemplateDecl *TD, const VarTemplateSpecializationDecl *D) {
> > -  // The specializations set is kept in the canonical template.
> > -  TD = TD->getCanonicalDecl();
> > -  if (!(!D->isFromASTFile() && TD->isFromASTFile()))
> > -    return; // Not a source specialization added to a template from PCH.
> > -
> > -  assert(!WritingAST && "Already writing the AST!");
> > -
> DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,
> > -                                       D));
> > -}
> > -
> > -void ASTWriter::AddedCXXTemplateSpecialization(const
> FunctionTemplateDecl *TD,
> > -                                               const FunctionDecl *D) {
> > -  // The specializations set is kept in the canonical template.
> > -  TD = TD->getCanonicalDecl();
> > -  if (!(!D->isFromASTFile() && TD->isFromASTFile()))
> > -    return; // Not a source specialization added to a template from PCH.
> > -
> > -  assert(!WritingAST && "Already writing the AST!");
> > -
> DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,
> > -                                       D));
> > -}
> > -
> >  void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) {
> >    assert(!DoneWritingDeclsAndTypes && "Already done writing updates!");
> >    if (!Chain) return;
> >
> > Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=245779&r1=245778&r2=245779&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
> > +++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Fri Aug 21 20:47:18
> 2015
> > @@ -159,6 +159,19 @@ namespace clang {
> >        Writer.AddStmt(FD->getBody());
> >      }
> >
> > +    /// Add to the record the first declaration from each module file
> that
> > +    /// provides a declaration of D. The intent is to provide a
> sufficient
> > +    /// set such that reloading this set will load all current
> redeclarations.
> > +    void AddFirstDeclFromEachModule(const Decl *D, bool IncludeLocal) {
> > +      llvm::MapVector<ModuleFile*, const Decl*> Firsts;
> > +      // FIXME: We can skip entries that we know are implied by others.
> > +      for (const Decl *R = D->getMostRecentDecl(); R; R =
> R->getPreviousDecl())
> > +        if (IncludeLocal || R->isFromASTFile())
> > +          Firsts[Writer.Chain->getOwningModuleFile(R)] = R;
> > +      for (const auto &F : Firsts)
> > +        Writer.AddDeclRef(F.second, Record);
> > +    }
> > +
> >      /// Get the specialization decl from an entry in the specialization
> list.
> >      template <typename EntryType>
> >      typename
> RedeclarableTemplateDecl::SpecEntryTraits<EntryType>::DeclType *
> > @@ -194,20 +207,46 @@ namespace clang {
> >        if (auto *LS = Common->LazySpecializations)
> >          LazySpecializations = ArrayRef<DeclID>(LS + 1, LS + 1 + LS[0]);
> >
> > -      Record.push_back(Specializations.size() +
> > -                       PartialSpecializations.size() +
> > -                       LazySpecializations.size());
> > +      // Add a slot to the record for the number of specializations.
> > +      unsigned I = Record.size();
> > +      Record.push_back(0);
> > +
> >        for (auto &Entry : Specializations) {
> >          auto *D = getSpecializationDecl(Entry);
> >          assert(D->isCanonicalDecl() && "non-canonical decl in set");
> > -        Writer.AddDeclRef(D, Record);
> > +        AddFirstDeclFromEachModule(D, /*IncludeLocal*/true);
> >        }
> >        for (auto &Entry : PartialSpecializations) {
> >          auto *D = getSpecializationDecl(Entry);
> >          assert(D->isCanonicalDecl() && "non-canonical decl in set");
> > -        Writer.AddDeclRef(D, Record);
> > +        AddFirstDeclFromEachModule(D, /*IncludeLocal*/true);
> >        }
> >        Record.append(LazySpecializations.begin(),
> LazySpecializations.end());
> > +
> > +      // Update the size entry we added earlier.
> > +      Record[I] = Record.size() - I - 1;
> > +    }
> > +
> > +    /// Ensure that this template specialization is associated with the
> specified
> > +    /// template on reload.
> > +    void RegisterTemplateSpecialization(const Decl *Template,
> > +                                        const Decl *Specialization) {
> > +      Template = Template->getCanonicalDecl();
> > +
> > +      // If the canonical template is local, we'll write out this
> specialization
> > +      // when we emit it.
> > +      // FIXME: We can do the same thing if there is any local
> declaration of
> > +      // the template, to avoid emitting an update record.
> > +      if (!Template->isFromASTFile())
> > +        return;
> > +
> > +      // We only need to associate the first local declaration of the
> > +      // specialization. The other declarations will get pulled in by
> it.
> > +      if (Writer.getFirstLocalDecl(Specialization) != Specialization)
> > +        return;
> > +
> > +      Writer.DeclUpdates[Template].push_back(ASTWriter::DeclUpdate(
> > +          UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION, Specialization));
> >      }
> >    };
> >  }
> > @@ -479,6 +518,9 @@ void ASTDeclWriter::VisitFunctionDecl(Fu
> >    case FunctionDecl::TK_FunctionTemplateSpecialization: {
> >      FunctionTemplateSpecializationInfo *
> >        FTSInfo = D->getTemplateSpecializationInfo();
> > +
> > +    RegisterTemplateSpecialization(FTSInfo->getTemplate(), D);
> > +
> >      Writer.AddDeclRef(FTSInfo->getTemplate(), Record);
> >      Record.push_back(FTSInfo->getTemplateSpecializationKind());
> >
> > @@ -1249,6 +1291,8 @@ void ASTDeclWriter::VisitClassTemplateDe
> >
> >  void ASTDeclWriter::VisitClassTemplateSpecializationDecl(
> >
>  ClassTemplateSpecializationDecl *D) {
> > +  RegisterTemplateSpecialization(D->getSpecializedTemplate(), D);
> > +
> >    VisitCXXRecordDecl(D);
> >
> >    llvm::PointerUnion<ClassTemplateDecl *,
> > @@ -1308,6 +1352,8 @@ void ASTDeclWriter::VisitVarTemplateDecl
> >
> >  void ASTDeclWriter::VisitVarTemplateSpecializationDecl(
> >      VarTemplateSpecializationDecl *D) {
> > +  RegisterTemplateSpecialization(D->getSpecializedTemplate(), D);
> > +
> >    VisitVarDecl(D);
> >
> >    llvm::PointerUnion<VarTemplateDecl *,
> VarTemplatePartialSpecializationDecl *>
> > @@ -1478,48 +1524,61 @@ void ASTDeclWriter::VisitDeclContext(Dec
> >    Record.push_back(VisibleOffset);
> >  }
> >
> > -/// Determine whether D is the first declaration in its redeclaration
> chain that
> > -/// is not from an AST file.
> > -template <typename T>
> > -static bool isFirstLocalDecl(Redeclarable<T> *D) {
> > -  assert(D && !static_cast<T*>(D)->isFromASTFile());
> > -  do
> > -    D = D->getPreviousDecl();
> > -  while (D && static_cast<T*>(D)->isFromASTFile());
> > -  return !D;
> > +/// \brief Is this a local declaration (that is, one that will be
> written to
> > +/// our AST file)? This is the case for declarations that are neither
> imported
> > +/// from another AST file nor predefined.
> > +static bool isLocalDecl(ASTWriter &W, const Decl *D) {
> > +  if (D->isFromASTFile())
> > +    return false;
> > +  return W.getDeclID(D) >= NUM_PREDEF_DECL_IDS;
> > +}
> > +
> > +const Decl *ASTWriter::getFirstLocalDecl(const Decl *D) {
> > +  assert(isLocalDecl(*this, D) && "expected a local declaration");
> > +
> > +  const Decl *Canon = D->getCanonicalDecl();
> > +  if (isLocalDecl(*this, Canon))
> > +    return Canon;
> > +
> > +  const Decl *&CacheEntry = FirstLocalDeclCache[Canon];
> > +  if (CacheEntry)
> > +    return CacheEntry;
> > +
> > +  for (const Decl *Redecl = D; Redecl; Redecl =
> Redecl->getPreviousDecl())
> > +    if (isLocalDecl(*this, Redecl))
> > +      D = Redecl;
> > +  return CacheEntry = D;
> >  }
> >
> >  template <typename T>
> >  void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {
> >    T *First = D->getFirstDecl();
> >    T *MostRecent = First->getMostRecentDecl();
> > +  T *DAsT = static_cast<T *>(D);
> >    if (MostRecent != First) {
> > -    assert(isRedeclarableDeclKind(static_cast<T *>(D)->getKind()) &&
> > +    assert(isRedeclarableDeclKind(DAsT->getKind()) &&
> >             "Not considered redeclarable?");
> >
> >      Writer.AddDeclRef(First, Record);
> >
> > -    // In a modules build, emit a list of all imported key declarations
> > -    // (excluding First, if it was imported), so that we can be sure
> that all
> > -    // redeclarations visible to this module are before D in the redecl
> chain.
> > -    unsigned I = Record.size();
> > -    Record.push_back(0);
> > -    if (Context.getLangOpts().Modules && Writer.Chain) {
> > -      if (isFirstLocalDecl(D)) {
> > -        Writer.Chain->forEachImportedKeyDecl(First, [&](const Decl *D) {
> > -          if (D != First)
> > -            Writer.AddDeclRef(D, Record);
> > -        });
> > -        Record[I] = Record.size() - I - 1;
> > -
> > -        // Write a redeclaration chain, attached to the first key decl.
> > -
> Writer.Redeclarations.push_back(Writer.Chain->getKeyDeclaration(First));
> > -      }
> > -    } else if (D == First || D->getPreviousDecl()->isFromASTFile()) {
> > -      assert(isFirstLocalDecl(D) && "imported decl after local decl");
> > -
> > -      // Write a redeclaration chain attached to the first decl.
> > -      Writer.Redeclarations.push_back(First);
> > +    // Write out a list of local redeclarations of this declaration if
> it's the
> > +    // first local declaration in the chain.
> > +    const Decl *FirstLocal = Writer.getFirstLocalDecl(DAsT);
> > +    if (DAsT == FirstLocal) {
> > +      Writer.Redeclarations.push_back(DAsT);
> > +
> > +      // Emit a list of all imported first declarations so that we can
> be sure
> > +      // that all redeclarations visible to this module are before D in
> the
> > +      // redecl chain.
> > +      unsigned I = Record.size();
> > +      Record.push_back(0);
> > +      if (Writer.Chain)
> > +        AddFirstDeclFromEachModule(DAsT, /*IncludeLocal*/false);
> > +      // This is the number of imported first declarations + 1.
> > +      Record[I] = Record.size() - I;
> > +    } else {
> > +      Record.push_back(0);
> > +      Writer.AddDeclRef(FirstLocal, Record);
> >      }
> >
> >      // Make sure that we serialize both the previous and the most-recent
> >
> > Modified: cfe/trunk/test/Modules/cxx-templates.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=245779&r1=245778&r2=245779&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/test/Modules/cxx-templates.cpp (original)
> > +++ cfe/trunk/test/Modules/cxx-templates.cpp Fri Aug 21 20:47:18 2015
> > @@ -187,10 +187,10 @@ namespace Std {
> >
> >  // CHECK-DUMP:      ClassTemplateDecl {{.*}}
> <{{.*[/\\]}}cxx-templates-common.h:1:1, {{.*}}>  col:{{.*}} in
> cxx_templates_common SomeTemplate
> >  // CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev
> {{.*}} SomeTemplate
> > -// CHECK-DUMP-NEXT:     TemplateArgument type 'char [2]'
> > -// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}}
> SomeTemplate definition
> > -// CHECK-DUMP-NEXT:     TemplateArgument type 'char [2]'
> > -// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev
> {{.*}} SomeTemplate
> >  // CHECK-DUMP-NEXT:     TemplateArgument type 'char [1]'
> >  // CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}}
> SomeTemplate definition
> >  // CHECK-DUMP-NEXT:     TemplateArgument type 'char [1]'
> > +// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}} prev
> {{.*}} SomeTemplate
> > +// CHECK-DUMP-NEXT:     TemplateArgument type 'char [2]'
> > +// CHECK-DUMP:        ClassTemplateSpecializationDecl {{.*}}
> SomeTemplate definition
> > +// CHECK-DUMP-NEXT:     TemplateArgument type 'char [2]'
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150827/ccef8bed/attachment-0001.html>


More information about the cfe-commits mailing list