r267691 - [modules] Fix Decl's Used invariant.
Vassil Vassilev via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 27 03:46:06 PDT 2016
Author: vvassilev
Date: Wed Apr 27 05:46:06 2016
New Revision: 267691
URL: http://llvm.org/viewvc/llvm-project?rev=267691&view=rev
Log:
[modules] Fix Decl's Used invariant.
The Decl::isUsed has a value for every decl. In non-module builds it is very
difficult (but possible) to break this invariant but when we walk up the redecl
chain we find the neccessary information.
When deserializing the decls from a module it is much more difficult to update
correctly this invariant. The patch centralizes the information whether a decl
is used in the canonical decl marking the entire entity as being used.
Fixes https://llvm.org/bugs/show_bug.cgi?id=27401
Patch by Cristina Cristescu and me.
Thanks to Richard Smith who helped to debug and understand the issue!
Reviewed by Richard Smith.
Added:
cfe/trunk/test/Modules/Inputs/PR27401/
cfe/trunk/test/Modules/Inputs/PR27401/a.h
cfe/trunk/test/Modules/Inputs/PR27401/b.h
cfe/trunk/test/Modules/Inputs/PR27401/module.modulemap
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=267691&r1=267690&r2=267691&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Wed Apr 27 05:46:06 2016
@@ -518,8 +518,8 @@ public:
bool isImplicit() const { return Implicit; }
void setImplicit(bool I = true) { Implicit = I; }
- /// \brief Whether this declaration was used, meaning that a definition
- /// is required.
+ /// \brief Whether *any* (re-)declaration of the entity 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,7 +529,8 @@ public:
/// \brief Set whether the declaration is used, in the sense of odr-use.
///
/// This should only be used immediately after creating a declaration.
- void setIsUsed() { Used = true; }
+ /// It intentionally doesn't notify any listeners.
+ void setIsUsed() { getCanonicalDecl()->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=267691&r1=267690&r2=267691&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Wed Apr 27 05:46:06 2016
@@ -565,6 +565,17 @@ 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=267691&r1=267690&r2=267691&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Wed Apr 27 05:46:06 2016
@@ -340,25 +340,29 @@ unsigned Decl::getMaxAlignment() const {
return Align;
}
-bool Decl::isUsed(bool CheckUsedAttr) const {
- if (Used)
+bool Decl::isUsed(bool CheckUsedAttr) const {
+ const Decl *CanonD = getCanonicalDecl();
+ if (CanonD->Used)
return true;
-
+
// Check for used attribute.
- if (CheckUsedAttr && hasAttr<UsedAttr>())
+ // Ask the most recent decl, since attributes accumulate in the redecl chain.
+ if (CheckUsedAttr && getMostRecentDecl()->hasAttr<UsedAttr>())
return true;
- return false;
+ // The information may have not been deserialized yet. Force deserialization
+ // to complete the needed information.
+ return getMostRecentDecl()->getCanonicalDecl()->Used;
}
void Decl::markUsed(ASTContext &C) {
- if (Used)
+ if (isUsed())
return;
if (C.getASTMutationListener())
C.getASTMutationListener()->DeclarationMarkedUsed(this);
- Used = true;
+ setIsUsed();
}
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=267691&r1=267690&r2=267691&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Apr 27 05:46:06 2016
@@ -13011,17 +13011,7 @@ void Sema::MarkFunctionReferenced(Source
UndefinedButUsed.insert(std::make_pair(Func->getCanonicalDecl(), Loc));
}
- // 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;
- }
+ Func->markUsed(Context);
}
static void
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=267691&r1=267690&r2=267691&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Apr 27 05:46:06 2016
@@ -51,6 +51,11 @@ 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) {
@@ -217,7 +222,8 @@ 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) {}
+ TypedefNameForLinkage(nullptr), HasPendingBody(false),
+ IsDeclMarkedUsed(false) {}
template <typename DeclT>
static Decl *getMostRecentDeclImpl(Redeclarable<DeclT> *D);
@@ -444,6 +450,11 @@ 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 =
@@ -524,6 +535,7 @@ 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++]);
@@ -548,7 +560,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);
@@ -2355,6 +2367,8 @@ 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
@@ -3112,11 +3126,6 @@ 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))
@@ -3865,7 +3874,7 @@ void ASTDeclReader::UpdateDecl(Decl *D,
// ASTMutationListeners other than an ASTWriter.
// Maintain AST consistency: any later redeclarations are used too.
- forAllLaterRedecls(D, [](Decl *D) { D->Used = true; });
+ D->setIsUsed();
break;
}
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=267691&r1=267690&r2=267691&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Apr 27 05:46:06 2016
@@ -5784,8 +5784,13 @@ void ASTWriter::AddedObjCCategoryToInter
void ASTWriter::DeclarationMarkedUsed(const Decl *D) {
assert(!WritingAST && "Already writing the AST!");
- if (!D->isFromASTFile())
- return;
+
+ // 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;
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=267691&r1=267690&r2=267691&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Wed Apr 27 05:46:06 2016
@@ -1541,16 +1541,6 @@ 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();
Added: cfe/trunk/test/Modules/Inputs/PR27401/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27401/a.h?rev=267691&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/PR27401/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/PR27401/a.h Wed Apr 27 05:46:06 2016
@@ -0,0 +1,17 @@
+#ifndef _LIBCPP_ALGORITHM
+#define _LIBCPP_ALGORITHM
+template <class _Tp, _Tp>
+struct integral_constant {
+ static const _Tp value = _Tp();
+};
+
+template <class _Tp>
+struct is_nothrow_default_constructible
+ : integral_constant<bool, __is_constructible(_Tp)> {};
+
+template <class _Tp>
+struct is_nothrow_move_constructible
+ : integral_constant<bool, __is_constructible(_Tp, _Tp)> {};
+
+class allocator {};
+#endif
Added: cfe/trunk/test/Modules/Inputs/PR27401/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27401/b.h?rev=267691&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/PR27401/b.h (added)
+++ cfe/trunk/test/Modules/Inputs/PR27401/b.h Wed Apr 27 05:46:06 2016
@@ -0,0 +1,21 @@
+#include "a.h"
+#ifndef _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);
+};
+
+#endif
+void GetUniquePtrType() { vector<char> v; }
+
Added: cfe/trunk/test/Modules/Inputs/PR27401/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR27401/module.modulemap?rev=267691&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/PR27401/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/PR27401/module.modulemap Wed Apr 27 05:46:06 2016
@@ -0,0 +1 @@
+module "b" { header "b.h" export * }
Added: cfe/trunk/test/Modules/pr27401.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr27401.cpp?rev=267691&view=auto
==============================================================================
--- cfe/trunk/test/Modules/pr27401.cpp (added)
+++ cfe/trunk/test/Modules/pr27401.cpp Wed Apr 27 05:46:06 2016
@@ -0,0 +1,38 @@
+// 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
More information about the cfe-commits
mailing list