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