r230121 - Revert r167816 and replace it with a proper fix for the issue: do not
Richard Smith
richard-llvm at metafoo.co.uk
Fri Feb 20 18:31:57 PST 2015
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);
}
More information about the cfe-commits
mailing list