I'd reland with the fix, and with an additional test that catches the problem this introduced. On Apr 28, 2016 8:16 AM, "Vassil Vassilev via cfe-commits" < cfe-commits@lists.llvm.org> wrote:
> Hi Nico, > I have a fix. What is the right way of putting it in? Shall I revert the > "revert" and commit the fix afterwards? > Many thanks > Vassil > On 27/04/16 19:26, Nico Weber via cfe-commits wrote: > >> Author: nico >> Date: Wed Apr 27 12:26:08 2016 >> New Revision: 267744 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=267744&view=rev >> Log: >> Revert r267691, it caused PR27535. >> >> Removed: >> cfe/trunk/test/Modules/Inputs/PR27401/ >> cfe/trunk/test/Modules/pr27401.cpp >> Modified: >> cfe/trunk/include/clang/AST/DeclBase.h >> cfe/trunk/include/clang/Serialization/ASTWriter.h >> cfe/trunk/lib/AST/DeclBase.cpp >> cfe/trunk/lib/Sema/SemaExpr.cpp >> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp >> cfe/trunk/lib/Serialization/ASTWriter.cpp >> cfe/trunk/lib/Serialization/ASTWriterDecl.cpp >> >> Modified: cfe/trunk/include/clang/AST/DeclBase.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=267744&r1=267743&r2=267744&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/AST/DeclBase.h (original) >> +++ cfe/trunk/include/clang/AST/DeclBase.h Wed Apr 27 12:26:08 2016 >> @@ -518,8 +518,8 @@ public: >> bool isImplicit() const { return Implicit; } >> void setImplicit(bool I = true) { Implicit = I; } >> - /// \brief Whether *any* (re-)declaration of the entity was used, >> meaning that >> - /// a definition is required. >> + /// \brief Whether this declaration was used, meaning that a definition >> + /// is required. >> /// >> /// \param CheckUsedAttr When true, also consider the "used" attribute >> /// (in addition to the "used" bit set by \c setUsed()) when >> determining >> @@ -529,8 +529,7 @@ public: >> /// \brief Set whether the declaration is used, in the sense of >> odr-use. >> /// >> /// This should only be used immediately after creating a declaration. >> - /// It intentionally doesn't notify any listeners. >> - void setIsUsed() { getCanonicalDecl()->Used = true; } >> + void setIsUsed() { Used = true; } >> /// \brief Mark the declaration used, in the sense of odr-use. >> /// >> >> Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=267744&r1=267743&r2=267744&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original) >> +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Wed Apr 27 12:26:08 >> 2016 >> @@ -565,17 +565,6 @@ public: >> /// decl. >> const Decl *getFirstLocalDecl(const Decl *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. >> - bool IsLocalDecl(const Decl *D) { >> - if (D->isFromASTFile()) >> - return false; >> - auto I = DeclIDs.find(D); >> - return (I == DeclIDs.end() || >> - I->second >= serialization::NUM_PREDEF_DECL_IDS); >> - }; >> - >> /// \brief Emit a reference to a declaration. >> void AddDeclRef(const Decl *D, RecordDataImpl &Record); >> >> Modified: cfe/trunk/lib/AST/DeclBase.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=267744&r1=267743&r2=267744&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/AST/DeclBase.cpp (original) >> +++ cfe/trunk/lib/AST/DeclBase.cpp Wed Apr 27 12:26:08 2016 >> @@ -340,29 +340,25 @@ unsigned Decl::getMaxAlignment() const { >> return Align; >> } >> -bool Decl::isUsed(bool CheckUsedAttr) const { >> - const Decl *CanonD = getCanonicalDecl(); >> - if (CanonD->Used) >> +bool Decl::isUsed(bool CheckUsedAttr) const { >> + if (Used) >> return true; >> - >> + >> // Check for used attribute. >> - // Ask the most recent decl, since attributes accumulate in the redecl >> chain. >> - if (CheckUsedAttr && getMostRecentDecl()->hasAttr<UsedAttr>()) >> + if (CheckUsedAttr && hasAttr<UsedAttr>()) >> return true; >> - // The information may have not been deserialized yet. Force >> deserialization >> - // to complete the needed information. >> - return getMostRecentDecl()->getCanonicalDecl()->Used; >> + return false; >> } >> void Decl::markUsed(ASTContext &C) { >> - if (isUsed()) >> + if (Used) >> return; >> if (C.getASTMutationListener()) >> C.getASTMutationListener()->DeclarationMarkedUsed(this); >> - setIsUsed(); >> + Used = true; >> } >> bool Decl::isReferenced() const { >> >> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=267744&r1=267743&r2=267744&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Apr 27 12:26:08 2016 >> @@ -13011,7 +13011,17 @@ void Sema::MarkFunctionReferenced(Source >> UndefinedButUsed.insert(std::make_pair(Func->getCanonicalDecl(), >> Loc)); >> } >> - Func->markUsed(Context); >> + // Normally the most current decl is marked used while processing the >> use and >> + // any subsequent decls are marked used by decl merging. This fails >> with >> + // template instantiation since marking can happen at the end of the >> file >> + // and, because of the two phase lookup, this function is called with >> at >> + // decl in the middle of a decl chain. We loop to maintain the >> invariant >> + // that once a decl is used, all decls after it are also used. >> + for (FunctionDecl *F = Func->getMostRecentDecl();; F = >> F->getPreviousDecl()) { >> + F->markUsed(Context); >> + if (F == Func) >> + break; >> + } >> } >> static void >> >> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=267744&r1=267743&r2=267744&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) >> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Apr 27 12:26:08 2016 >> @@ -51,11 +51,6 @@ namespace clang { >> bool HasPendingBody; >> - ///\brief A flag to carry the information for a decl from the >> entity is >> - /// used. We use it to delay the marking of the canonical decl as >> used until >> - /// the entire declaration is deserialized and merged. >> - bool IsDeclMarkedUsed; >> - >> uint64_t GetCurrentCursorOffset(); >> uint64_t ReadLocalOffset(const RecordData &R, unsigned &I) { >> @@ -222,8 +217,7 @@ namespace clang { >> : Reader(Reader), F(*Loc.F), Offset(Loc.Offset), >> ThisDeclID(thisDeclID), >> ThisDeclLoc(ThisDeclLoc), Record(Record), Idx(Idx), >> TypeIDForTypeDecl(0), NamedDeclForTagDecl(0), >> - TypedefNameForLinkage(nullptr), HasPendingBody(false), >> - IsDeclMarkedUsed(false) {} >> + TypedefNameForLinkage(nullptr), HasPendingBody(false) {} >> template <typename DeclT> >> static Decl *getMostRecentDeclImpl(Redeclarable<DeclT> *D); >> @@ -450,11 +444,6 @@ uint64_t ASTDeclReader::GetCurrentCursor >> void ASTDeclReader::Visit(Decl *D) { >> DeclVisitor<ASTDeclReader, void>::Visit(D); >> - // At this point we have deserialized and merged the decl and it is >> safe to >> - // update its canonical decl to signal that the entire entity is used. >> - D->getCanonicalDecl()->Used |= IsDeclMarkedUsed; >> - IsDeclMarkedUsed = false; >> - >> if (DeclaratorDecl *DD = dyn_cast<DeclaratorDecl>(D)) { >> if (DD->DeclInfo) { >> DeclaratorDecl::ExtInfo *Info = >> @@ -535,7 +524,6 @@ void ASTDeclReader::VisitDecl(Decl *D) { >> } >> D->setImplicit(Record[Idx++]); >> D->Used = Record[Idx++]; >> - IsDeclMarkedUsed |= D->Used; >> D->setReferenced(Record[Idx++]); >> D->setTopLevelDeclInObjCContainer(Record[Idx++]); >> D->setAccess((AccessSpecifier)Record[Idx++]); >> @@ -560,7 +548,7 @@ void ASTDeclReader::VisitDecl(Decl *D) { >> if (Owner->NameVisibility != Module::AllVisible) { >> // The owning module is not visible. Mark this declaration as >> hidden. >> D->Hidden = true; >> - >> + >> // Note that this declaration was hidden because its owning >> module is >> // not yet visible. >> Reader.HiddenNamesMap[Owner].push_back(D); >> @@ -2367,8 +2355,6 @@ void ASTDeclReader::mergeRedeclarable(Re >> // appropriate canonical declaration. >> D->RedeclLink = Redeclarable<T>::PreviousDeclLink(ExistingCanon); >> D->First = ExistingCanon; >> - ExistingCanon->Used |= D->Used; >> - D->Used = false; >> // When we merge a namespace, update its pointer to the first >> namespace. >> // We cannot have loaded any redeclarations of this declaration >> yet, so >> @@ -3126,6 +3112,11 @@ void ASTDeclReader::attachPreviousDecl(A >> Previous->IdentifierNamespace & >> (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type); >> + // If the previous declaration is marked as used, then this >> declaration should >> + // be too. >> + if (Previous->Used) >> + D->Used = true; >> + >> // If the declaration declares a template, it may inherit default >> arguments >> // from the previous declaration. >> if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D)) >> @@ -3874,7 +3865,7 @@ void ASTDeclReader::UpdateDecl(Decl *D, >> // ASTMutationListeners other than an ASTWriter. >> // Maintain AST consistency: any later redeclarations are used >> too. >> - D->setIsUsed(); >> + forAllLaterRedecls(D, [](Decl *D) { D->Used = true; }); >> break; >> } >> >> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=267744&r1=267743&r2=267744&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) >> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Apr 27 12:26:08 2016 >> @@ -5784,13 +5784,8 @@ void ASTWriter::AddedObjCCategoryToInter >> void ASTWriter::DeclarationMarkedUsed(const Decl *D) { >> assert(!WritingAST && "Already writing the AST!"); >> - >> - // If there is *any* declaration of the entity that's not from an AST >> file, >> - // we can skip writing the update record. We make sure that isUsed() >> triggers >> - // completion of the redeclaration chain of the entity. >> - for (auto Prev = D->getMostRecentDecl(); Prev; Prev = >> Prev->getPreviousDecl()) >> - if (IsLocalDecl(Prev)) >> - return; >> + if (!D->isFromASTFile()) >> + return; >> DeclUpdates[D].push_back(DeclUpdate(UPD_DECL_MARKED_USED)); >> } >> >> Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=267744&r1=267743&r2=267744&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original) >> +++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Wed Apr 27 12:26:08 2016 >> @@ -1541,6 +1541,16 @@ void ASTDeclWriter::VisitDeclContext(Dec >> } >> const Decl *ASTWriter::getFirstLocalDecl(const Decl *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. >> + auto IsLocalDecl = [&](const Decl *D) -> bool { >> + if (D->isFromASTFile()) >> + return false; >> + auto I = DeclIDs.find(D); >> + return (I == DeclIDs.end() || I->second >= NUM_PREDEF_DECL_IDS); >> + }; >> + >> assert(IsLocalDecl(D) && "expected a local declaration"); >> const Decl *Canon = D->getCanonicalDecl(); >> >> Removed: cfe/trunk/test/Modules/pr27401.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr27401.cpp?rev=267743&view=auto >> >> ============================================================================== >> --- cfe/trunk/test/Modules/pr27401.cpp (original) >> +++ cfe/trunk/test/Modules/pr27401.cpp (removed) >> @@ -1,38 +0,0 @@ >> -// RUN: rm -rf %t >> -// RUN: %clang_cc1 -std=c++11 -I%S/Inputs/PR27401 -verify %s >> -// RUN: %clang_cc1 -std=c++11 -fmodules >> -fmodule-map-file=%S/Inputs/PR27401/module.modulemap >> -fmodules-cache-path=%t -I%S/Inputs/PR27401 -verify %s >> - >> -#include "a.h" >> -#define _LIBCPP_VECTOR >> -template <class, class _Allocator> >> -class __vector_base { >> -protected: >> - _Allocator __alloc() const; >> - __vector_base(_Allocator); >> -}; >> - >> -template <class _Tp, class _Allocator = allocator> >> -class vector : __vector_base<_Tp, _Allocator> { >> -public: >> - vector() noexcept(is_nothrow_default_constructible<_Allocator>::value); >> - vector(const vector &); >> - vector(vector &&) >> - noexcept(is_nothrow_move_constructible<_Allocator>::value); >> -}; >> - >> -template <class _Tp, class _Allocator> >> -vector<_Tp, _Allocator>::vector(const vector &__x) : __vector_base<_Tp, >> _Allocator>(__x.__alloc()) {} >> - >> - struct CommentOptions { >> - vector<char> ParseAllComments; >> - CommentOptions() {} >> - }; >> - struct PrintingPolicy { >> - PrintingPolicy(CommentOptions LO) : LangOpts(LO) {} >> - CommentOptions LangOpts; >> - }; >> - >> -#include "b.h" >> -CommentOptions fn1() { return fn1(); } >> - >> -// expected-no-diagnostics >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
