<div dir="ltr">Note that this leads to a crash in ResolveConstructorOverload where Ctors has an invalid pointer. Investigating.<br></div><br><div class="gmail_quote">On Sat Feb 21 2015 at 3:37:51 AM Richard Smith <<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rsmith<br>
Date: Fri Feb 20 20:31:57 2015<br>
New Revision: 230121<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=230121&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=230121&view=rev</a><br>
Log:<br>
Revert r167816 and replace it with a proper fix for the issue: do not<br>
invalidate lookup_iterators and lookup_results for some name within a<br>
DeclContext if the lookup results for a *different* name change.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/AST/<u></u>DeclBase.h<br>
cfe/trunk/include/clang/AST/<u></u>DeclContextInternals.h<br>
cfe/trunk/lib/AST/DeclBase.cpp<br>
cfe/trunk/lib/AST/DeclCXX.cpp<br>
cfe/trunk/lib/Sema/SemaInit.<u></u>cpp<br>
cfe/trunk/lib/Serialization/<u></u>ASTWriter.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/<u></u>DeclBase.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=230121&r1=230120&r2=230121&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/include/<u></u>clang/AST/DeclBase.h?rev=<u></u>230121&r1=230120&r2=230121&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/include/clang/AST/<u></u>DeclBase.h (original)<br>
+++ cfe/trunk/include/clang/AST/<u></u>DeclBase.h Fri Feb 20 20:31:57 2015<br>
@@ -18,6 +18,7 @@<br>
#include "clang/AST/DeclarationName.h"<br>
#include "clang/Basic/Specifiers.h"<br>
#include "llvm/ADT/PointerUnion.h"<br>
+#include "llvm/ADT/iterator.h"<br>
#include "llvm/ADT/iterator_range.h"<br>
#include "llvm/Support/Compiler.h"<br>
#include "llvm/Support/<u></u>PrettyStackTrace.h"<br>
@@ -1005,9 +1006,65 @@ public:<br>
void print(raw_ostream &OS) const override;<br>
};<br>
<br>
-typedef MutableArrayRef<NamedDecl *> DeclContextLookupResult;<br>
+/// \brief The results of name lookup within a DeclContext. This is either a<br>
+/// single result (with no stable storage) or a collection of results (with<br>
+/// stable storage provided by the lookup table).<br>
+class DeclContextLookupResult {<br>
+ typedef ArrayRef<NamedDecl *> ResultTy;<br>
+ ResultTy Result;<br>
+ // If there is only one lookup result, it would be invalidated by<br>
+ // reallocations of the name table, so store it separately.<br>
+ NamedDecl *Single;<br>
<br>
-typedef ArrayRef<NamedDecl *> DeclContextLookupConstResult;<br>
+ static NamedDecl *const SingleElementDummyList;<br>
+<br>
+public:<br>
+ DeclContextLookupResult() : Result(), Single() {}<br>
+ DeclContextLookupResult(<u></u>ArrayRef<NamedDecl *> Result)<br>
+ : Result(Result), Single() {}<br>
+ DeclContextLookupResult(<u></u>NamedDecl *Single)<br>
+ : Result(SingleElementDummyList)<u></u>, Single(Single) {}<br>
+<br>
+ class iterator;<br>
+ typedef llvm::iterator_adaptor_base<<u></u>iterator, ResultTy::iterator,<br>
+ std::random_access_iterator_<u></u>tag,<br>
+ NamedDecl *const> IteratorBase;<br>
+ class iterator : public IteratorBase {<br>
+ value_type SingleElement;<br>
+<br>
+ public:<br>
+ iterator() : IteratorBase(), SingleElement() {}<br>
+ explicit iterator(pointer Pos, value_type Single = nullptr)<br>
+ : IteratorBase(Pos), SingleElement(Single) {}<br>
+<br>
+ reference operator*() const {<br>
+ return SingleElement ? SingleElement : IteratorBase::operator*();<br>
+ }<br>
+ };<br>
+ typedef iterator const_iterator;<br>
+ typedef iterator::pointer pointer;<br>
+ typedef iterator::reference reference;<br>
+<br>
+ iterator begin() const { return iterator(Result.begin(), Single); }<br>
+ iterator end() const { return iterator(Result.end(), Single); }<br>
+<br>
+ bool empty() const { return Result.empty(); }<br>
+ pointer data() const { return Single ? &Single : Result.data(); }<br>
+ size_t size() const { return Single ? 1 : Result.size(); }<br>
+ reference front() const { return Single ? Single : Result.front(); }<br>
+ reference back() const { return Single ? Single : Result.back(); }<br>
+ reference operator[](size_t N) const { return Single ? Single : Result[N]; }<br>
+<br>
+ // FIXME: Remove this from the interface<br>
+ DeclContextLookupResult slice(size_t N) const {<br>
+ DeclContextLookupResult Sliced = Result.slice(N);<br>
+ Sliced.Single = Single;<br>
+ return Sliced;<br>
+ }<br>
+};<br>
+<br>
+// FIXME: Remove this.<br>
+typedef DeclContextLookupResult DeclContextLookupConstResult;<br>
<br>
/// DeclContext - This is used only as base class of specific decl types that<br>
/// can act as declaration contexts. These decls are (only the top classes<br>
@@ -1520,26 +1577,19 @@ public:<br>
/// @brief Checks whether a declaration is in this context.<br>
bool containsDecl(Decl *D) const;<br>
<br>
- /// lookup_iterator - An iterator that provides access to the results<br>
- /// of looking up a name within this context.<br>
- typedef NamedDecl **lookup_iterator;<br>
-<br>
- /// lookup_const_iterator - An iterator that provides non-mutable<br>
- /// access to the results of lookup up a name within this context.<br>
- typedef NamedDecl * const * lookup_const_iterator;<br>
-<br>
typedef DeclContextLookupResult lookup_result;<br>
- typedef DeclContextLookupConstResult lookup_const_result;<br>
+ typedef lookup_result::iterator lookup_iterator;<br>
+<br>
+ // FIXME: Remove these.<br>
+ typedef lookup_result lookup_const_result;<br>
+ typedef lookup_iterator lookup_const_iterator;<br>
<br>
/// lookup - Find the declarations (if any) with the given Name in<br>
/// this context. Returns a range of iterators that contains all of<br>
/// the declarations with this name, with object, function, member,<br>
/// and enumerator names preceding any tag name. Note that this<br>
/// routine will not look into parent contexts.<br>
- lookup_result lookup(DeclarationName Name);<br>
- lookup_const_result lookup(DeclarationName Name) const {<br>
- return const_cast<DeclContext*>(this)<u></u>->lookup(Name);<br>
- }<br>
+ lookup_result lookup(DeclarationName Name) const;<br>
<br>
/// \brief Find the declarations with the given name that are visible<br>
/// within this context; don't attempt to retrieve anything from an<br>
@@ -1593,7 +1643,16 @@ public:<br>
all_lookups_iterator noload_lookups_begin() const;<br>
all_lookups_iterator noload_lookups_end() const;<br>
<br>
- typedef llvm::iterator_range<<u></u>UsingDirectiveDecl * const *> udir_range;<br>
+ struct udir_iterator;<br>
+ typedef llvm::iterator_adaptor_base<<u></u>udir_iterator, lookup_iterator,<br>
+ std::random_access_iterator_<u></u>tag,<br>
+ UsingDirectiveDecl *> udir_iterator_base;<br>
+ struct udir_iterator : udir_iterator_base {<br>
+ udir_iterator(lookup_iterator I) : udir_iterator_base(I) {}<br>
+ UsingDirectiveDecl *operator*() const;<br>
+ };<br>
+<br>
+ typedef llvm::iterator_range<udir_<u></u>iterator> udir_range;<br>
<br>
udir_range using_directives() const;<br>
<br>
<br>
Modified: cfe/trunk/include/clang/AST/<u></u>DeclContextInternals.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclContextInternals.h?rev=230121&r1=230120&r2=230121&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/include/<u></u>clang/AST/<u></u>DeclContextInternals.h?rev=<u></u>230121&r1=230120&r2=230121&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/include/clang/AST/<u></u>DeclContextInternals.h (original)<br>
+++ cfe/trunk/include/clang/AST/<u></u>DeclContextInternals.h Fri Feb 20 20:31:57 2015<br>
@@ -142,23 +142,21 @@ public:<br>
/// represents.<br>
DeclContext::lookup_result getLookupResult() {<br>
if (isNull())<br>
- return DeclContext::lookup_result(<u></u>DeclContext::lookup_iterator(<u></u>nullptr),<br>
- DeclContext::lookup_iterator(<u></u>nullptr));<br>
+ return DeclContext::lookup_result();<br>
<br>
// If we have a single NamedDecl, return it.<br>
- if (getAsDecl()) {<br>
+ if (NamedDecl *ND = getAsDecl()) {<br>
assert(!isNull() && "Empty list isn't allowed");<br>
<br>
// Data is a raw pointer to a NamedDecl*, return it.<br>
- void *Ptr = &Data;<br>
- return DeclContext::lookup_result((<u></u>NamedDecl**)Ptr, (NamedDecl**)Ptr+1);<br>
+ return DeclContext::lookup_result(ND)<u></u>;<br>
}<br>
<br>
assert(getAsVector() && "Must have a vector at this point");<br>
DeclsTy &Vector = *getAsVector();<br>
<br>
// Otherwise, we have a range result.<br>
- return DeclContext::lookup_result(<u></u>Vector.begin(), Vector.end());<br>
+ return DeclContext::lookup_result(<u></u>Vector);<br>
}<br>
<br>
/// HandleRedeclaration - If this is a redeclaration of an existing decl,<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=230121&r1=230120&r2=230121&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/lib/AST/<u></u>DeclBase.cpp?rev=230121&r1=<u></u>230120&r2=230121&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/AST/DeclBase.cpp (original)<br>
+++ cfe/trunk/lib/AST/DeclBase.cpp Fri Feb 20 20:31:57 2015<br>
@@ -1297,12 +1297,14 @@ void DeclContext::buildLookupImpl(<u></u>DeclCo<br>
}<br>
}<br>
<br>
+NamedDecl *const DeclContextLookupResult::<u></u>SingleElementDummyList = nullptr;<br>
+<br>
DeclContext::lookup_result<br>
-DeclContext::lookup(<u></u>DeclarationName Name) {<br>
+DeclContext::lookup(<u></u>DeclarationName Name) const {<br>
assert(DeclKind != Decl::LinkageSpec &&<br>
"Should not perform lookups into linkage specs!");<br>
<br>
- DeclContext *PrimaryContext = getPrimaryContext();<br>
+ const DeclContext *PrimaryContext = getPrimaryContext();<br>
if (PrimaryContext != this)<br>
return PrimaryContext->lookup(Name);<br>
<br>
@@ -1318,7 +1320,8 @@ DeclContext::lookup(<u></u>DeclarationName Name<br>
StoredDeclsMap *Map = LookupPtr.getPointer();<br>
<br>
if (LookupPtr.getInt())<br>
- Map = buildLookup();<br>
+ // FIXME: Make buildLookup const?<br>
+ Map = const_cast<DeclContext*>(this)<u></u>->buildLookup();<br>
<br>
if (!Map)<br>
Map = CreateStoredDeclsMap(<u></u>getParentASTContext());<br>
@@ -1338,19 +1341,19 @@ DeclContext::lookup(<u></u>DeclarationName Name<br>
}<br>
}<br>
<br>
- return lookup_result(lookup_iterator(<u></u>nullptr), lookup_iterator(nullptr));<br>
+ return lookup_result();<br>
}<br>
<br>
StoredDeclsMap *Map = LookupPtr.getPointer();<br>
if (LookupPtr.getInt())<br>
- Map = buildLookup();<br>
+ Map = const_cast<DeclContext*>(this)<u></u>->buildLookup();<br>
<br>
if (!Map)<br>
- return lookup_result(lookup_iterator(<u></u>nullptr), lookup_iterator(nullptr));<br>
+ return lookup_result();<br>
<br>
StoredDeclsMap::iterator I = Map->find(Name);<br>
if (I == Map->end())<br>
- return lookup_result(lookup_iterator(<u></u>nullptr), lookup_iterator(nullptr));<br>
+ return lookup_result();<br>
<br>
return I->second.getLookupResult();<br>
}<br>
@@ -1387,12 +1390,11 @@ DeclContext::noload_lookup(<u></u>DeclarationNa<br>
}<br>
<br>
if (!Map)<br>
- return lookup_result(lookup_iterator(<u></u>nullptr), lookup_iterator(nullptr));<br>
+ return lookup_result();<br>
<br>
StoredDeclsMap::iterator I = Map->find(Name);<br>
return I != Map->end() ? I->second.getLookupResult()<br>
- : lookup_result(lookup_iterator(<u></u>nullptr),<br>
- lookup_iterator(nullptr));<br>
+ : lookup_result();<br>
}<br>
<br>
void DeclContext::<u></u>localUncachedLookup(<u></u>DeclarationName Name,<br>
@@ -1574,15 +1576,17 @@ void DeclContext::<u></u>makeDeclVisibleInConte<br>
DeclNameEntries.<u></u>AddSubsequentDecl(D);<br>
}<br>
<br>
+UsingDirectiveDecl *DeclContext::udir_iterator::<u></u>operator*() const {<br>
+ return cast<UsingDirectiveDecl>(*I);<br>
+}<br>
+<br>
/// Returns iterator range [First, Last) of UsingDirectiveDecls stored within<br>
/// this context.<br>
DeclContext::udir_range DeclContext::using_directives(<u></u>) const {<br>
// FIXME: Use something more efficient than normal lookup for using<br>
// directives. In C++, using directives are looked up more than anything else.<br>
lookup_const_result Result = lookup(UsingDirectiveDecl::<u></u>getName());<br>
- return udir_range(<br>
- reinterpret_cast<<u></u>UsingDirectiveDecl *const *>(Result.begin()),<br>
- reinterpret_cast<<u></u>UsingDirectiveDecl *const *>(Result.end()));<br>
+ return udir_range(Result.begin(), Result.end());<br>
}<br>
<br>
//===-------------------------<u></u>------------------------------<u></u>---------------===//<br>
<br>
Modified: cfe/trunk/lib/AST/DeclCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=230121&r1=230120&r2=230121&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/lib/AST/<u></u>DeclCXX.cpp?rev=230121&r1=<u></u>230120&r2=230121&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)<br>
+++ cfe/trunk/lib/AST/DeclCXX.cpp Fri Feb 20 20:31:57 2015<br>
@@ -1418,9 +1418,8 @@ CXXMethodDecl::<u></u>getCorrespondingMethodInC<br>
return nullptr;<br>
}<br>
<br>
- lookup_const_result Candidates = RD->lookup(getDeclName());<br>
- for (NamedDecl * const * I = Candidates.begin(); I != Candidates.end(); ++I) {<br>
- CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(*I);<br>
+ for (auto *ND : RD->lookup(getDeclName())) {<br>
+ CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(ND);<br>
if (!MD)<br>
continue;<br>
if (recursivelyOverrides(MD, this))<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaInit.<u></u>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=230121&r1=230120&r2=230121&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/lib/Sema/<u></u>SemaInit.cpp?rev=230121&r1=<u></u>230120&r2=230121&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/Sema/SemaInit.<u></u>cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaInit.<u></u>cpp Fri Feb 20 20:31:57 2015<br>
@@ -3171,15 +3171,13 @@ static OverloadingResult<br>
ResolveConstructorOverload(<u></u>Sema &S, SourceLocation DeclLoc,<br>
MultiExprArg Args,<br>
OverloadCandidateSet &CandidateSet,<br>
- ArrayRef<NamedDecl *> Ctors,<br>
+ DeclContext::lookup_result Ctors,<br>
OverloadCandidateSet::iterator &Best,<br>
bool CopyInitializing, bool AllowExplicit,<br>
bool OnlyListConstructors, bool IsListInit) {<br>
CandidateSet.clear();<br>
<br>
- for (ArrayRef<NamedDecl *>::iterator<br>
- Con = Ctors.begin(), ConEnd = Ctors.end(); Con != ConEnd; ++Con) {<br>
- NamedDecl *D = *Con;<br>
+ for (NamedDecl *D : Ctors) {<br>
DeclAccessPair FoundDecl = DeclAccessPair::make(D, D->getAccess());<br>
bool SuppressUserConversions = false;<br>
<br>
@@ -3281,11 +3279,7 @@ static void TryConstructorInitialization<br>
// - Otherwise, if T is a class type, constructors are considered. The<br>
// applicable constructors are enumerated, and the best one is chosen<br>
// through overload resolution.<br>
- DeclContext::lookup_result R = S.LookupConstructors(<u></u>DestRecordDecl);<br>
- // The container holding the constructors can under certain conditions<br>
- // be changed while iterating (e.g. because of deserialization).<br>
- // To be safe we copy the lookup results to a new container.<br>
- SmallVector<NamedDecl*, 16> Ctors(R.begin(), R.end());<br>
+ DeclContext::lookup_result Ctors = S.LookupConstructors(<u></u>DestRecordDecl);<br>
<br>
OverloadingResult Result = OR_No_Viable_Function;<br>
OverloadCandidateSet::iterator Best;<br>
@@ -3663,14 +3657,7 @@ static OverloadingResult TryRefInitWithC<br>
// to see if there is a suitable conversion.<br>
CXXRecordDecl *T1RecordDecl = cast<CXXRecordDecl>(<u></u>T1RecordType->getDecl());<br>
<br>
- DeclContext::lookup_result R = S.LookupConstructors(<u></u>T1RecordDecl);<br>
- // The container holding the constructors can under certain conditions<br>
- // be changed while iterating (e.g. because of deserialization).<br>
- // To be safe we copy the lookup results to a new container.<br>
- SmallVector<NamedDecl*, 16> Ctors(R.begin(), R.end());<br>
- for (SmallVectorImpl<NamedDecl *>::iterator<br>
- CI = Ctors.begin(), CE = Ctors.end(); CI != CE; ++CI) {<br>
- NamedDecl *D = *CI;<br>
+ for (NamedDecl *D : S.LookupConstructors(<u></u>T1RecordDecl)) {<br>
DeclAccessPair FoundDecl = DeclAccessPair::make(D, D->getAccess());<br>
<br>
// Find the constructor (which may be a template).<br>
<br>
Modified: cfe/trunk/lib/Serialization/<u></u>ASTWriter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=230121&r1=230120&r2=230121&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/lib/<u></u>Serialization/ASTWriter.cpp?<u></u>rev=230121&r1=230120&r2=<u></u>230121&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/Serialization/<u></u>ASTWriter.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/<u></u>ASTWriter.cpp Fri Feb 20 20:31:57 2015<br>
@@ -3756,16 +3756,14 @@ ASTWriter::<u></u>GenerateNameLookupTable(const<br>
// Add the constructors.<br>
if (!ConstructorDecls.empty()) {<br>
Generator.insert(<u></u>ConstructorName,<br>
- DeclContext::lookup_result(<u></u>ConstructorDecls.begin(),<br>
- ConstructorDecls.end()),<br>
+ DeclContext::lookup_result(<u></u>ConstructorDecls),<br>
Trait);<br>
}<br>
<br>
// Add the conversion functions.<br>
if (!ConversionDecls.empty()) {<br>
Generator.insert(<u></u>ConversionName,<br>
- DeclContext::lookup_result(<u></u>ConversionDecls.begin(),<br>
- ConversionDecls.end()),<br>
+ DeclContext::lookup_result(<u></u>ConversionDecls),<br>
Trait);<br>
}<br>
<br>
<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
</blockquote></div>