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