r230121 - Revert r167816 and replace it with a proper fix for the issue: do not
Manuel Klimek
klimek at google.com
Tue Feb 24 01:59:09 PST 2015
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 <richard-llvm at metafoo.co.uk>
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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150224/ed6220f3/attachment.html>
More information about the cfe-commits
mailing list