Skip to content

Commit

Permalink
Reland another version of "The Sema::LookupConstructor is not iterati…
Browse files Browse the repository at this point in the history
…on safe."

When looking up a ctor the modules infrasturcture deserializes more ctor
candidates in the body of the function causing the internal vector implementation
to rellocate and invalidate the pointers.

This patch makes Sema::LookupConstructor void and stabilizes the iteration.

This patch is a previous version of the patch in https://reviews.llvm.org/D91524
and we, after being merged, we should be able to backport it.
  • Loading branch information
vgvassilev committed Mar 4, 2021
1 parent 3cb3617 commit 6494f1f
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 20 deletions.
4 changes: 3 additions & 1 deletion core/metacling/src/TClingMethodInfo.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,9 @@ TClingMethodInfo::TClingMethodInfo(cling::Interpreter *interp,

// Assemble special functions (or FunctionTemplate-s) that are synthesized from DefinitionData but
// won't be enumerated as part of decls_begin()/decls_end().
for (clang::NamedDecl *ctor : SemaRef.LookupConstructors(CXXRD)) {
llvm::SmallVector<NamedDecl*, 16> Ctors;
SemaRef.LookupConstructors(CXXRD, Ctors);
for (clang::NamedDecl *ctor : Ctors) {
// Filter out constructor templates, they are not functions we can iterate over:
if (auto *CXXCD = llvm::dyn_cast<clang::CXXConstructorDecl>(ctor))
SpecFuncs.emplace_back(CXXCD);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class DependentDiagnostic;
/// one entry.
struct StoredDeclsList {
/// When in vector form, this is what the Data pointer points to.
using DeclsTy = SmallVector<NamedDecl *, 32>;
using DeclsTy = SmallVector<NamedDecl *, 8>;

/// A collection of declarations, with a flag to indicate if we have
/// further external declarations.
Expand Down
3 changes: 2 additions & 1 deletion interpreter/llvm/src/tools/clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -3463,7 +3463,8 @@ class Sema {
LabelDecl *LookupOrCreateLabel(IdentifierInfo *II, SourceLocation IdentLoc,
SourceLocation GnuLabelLoc = SourceLocation());

DeclContextLookupResult LookupConstructors(CXXRecordDecl *Class);
void LookupConstructors(CXXRecordDecl *Class,
llvm::SmallVectorImpl<NamedDecl*> &Constructors);
CXXConstructorDecl *LookupDefaultConstructor(CXXRecordDecl *Class);
CXXConstructorDecl *LookupCopyingConstructor(CXXRecordDecl *Class,
unsigned Quals);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5212,7 +5212,9 @@ QualType Sema::ProduceConstructorSignatureHelp(Scope *S, QualType Type,

OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);

for (NamedDecl *C : LookupConstructors(RD)) {
llvm::SmallVector<NamedDecl*, 4> Ctors;
LookupConstructors(RD, Ctors);
for (NamedDecl *C : Ctors) {
if (auto *FD = dyn_cast<FunctionDecl>(C)) {
AddOverloadCandidate(FD, DeclAccessPair::make(FD, C->getAccess()), Args,
CandidateSet,
Expand Down
4 changes: 3 additions & 1 deletion interpreter/llvm/src/tools/clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10162,7 +10162,9 @@ NamedDecl *Sema::BuildUsingDeclaration(
UsingName.setName(Context.DeclarationNames.getCXXConstructorName(
Context.getCanonicalType(Context.getRecordType(CurClass))));
UsingName.setNamedTypeInfo(nullptr);
for (auto *Ctor : LookupConstructors(RD))
llvm::SmallVector<NamedDecl*, 4> Ctors;
LookupConstructors(RD, Ctors);
for (auto *Ctor : Ctors)
R.addDecl(Ctor);
R.resolveKind();
} else {
Expand Down
8 changes: 6 additions & 2 deletions interpreter/llvm/src/tools/clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4804,7 +4804,9 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT,

bool FoundConstructor = false;
unsigned FoundTQs;
for (const auto *ND : Self.LookupConstructors(RD)) {
llvm::SmallVector<NamedDecl*, 4> Ctors;
Self.LookupConstructors(RD, Ctors);
for (const auto *ND : Ctors) {
// A template constructor is never a copy constructor.
// FIXME: However, it may actually be selected at the actual overload
// resolution point.
Expand Down Expand Up @@ -4845,7 +4847,9 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT,
return true;

bool FoundConstructor = false;
for (const auto *ND : Self.LookupConstructors(RD)) {
llvm::SmallVector<NamedDecl*, 4> Ctors;
Self.LookupConstructors(RD, Ctors);
for (const auto *ND : Ctors) {
// FIXME: In C++0x, a constructor template can be a default constructor.
if (isa<FunctionTemplateDecl>(ND->getUnderlyingDecl()))
continue;
Expand Down
20 changes: 13 additions & 7 deletions interpreter/llvm/src/tools/clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3735,7 +3735,7 @@ ResolveConstructorOverload(Sema &S, SourceLocation DeclLoc,
MultiExprArg Args,
OverloadCandidateSet &CandidateSet,
QualType DestType,
DeclContext::lookup_result Ctors,
const llvm::SmallVectorImpl<NamedDecl*>& Ctors,
OverloadCandidateSet::iterator &Best,
bool CopyInitializing, bool AllowExplicit,
bool OnlyListConstructors, bool IsListInit,
Expand Down Expand Up @@ -3915,7 +3915,8 @@ static void TryConstructorInitialization(Sema &S,
// - 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 Ctors = S.LookupConstructors(DestRecordDecl);
llvm::SmallVector<NamedDecl*, 4> Ctors;
S.LookupConstructors(DestRecordDecl, Ctors);

OverloadingResult Result = OR_No_Viable_Function;
OverloadCandidateSet::iterator Best;
Expand Down Expand Up @@ -4370,7 +4371,9 @@ static OverloadingResult TryRefInitWithConversionFunction(
// to see if there is a suitable conversion.
CXXRecordDecl *T1RecordDecl = cast<CXXRecordDecl>(T1RecordType->getDecl());

for (NamedDecl *D : S.LookupConstructors(T1RecordDecl)) {
llvm::SmallVector<NamedDecl*, 4> Ctors;
S.LookupConstructors(T1RecordDecl, Ctors);
for (NamedDecl *D : Ctors) {
auto Info = getConstructorInfo(D);
if (!Info.Constructor)
continue;
Expand Down Expand Up @@ -5014,7 +5017,9 @@ static void TryUserDefinedConversion(Sema &S,

// Try to complete the type we're converting to.
if (S.isCompleteType(Kind.getLocation(), DestType)) {
for (NamedDecl *D : S.LookupConstructors(DestRecordDecl)) {
llvm::SmallVector<NamedDecl*, 4> Ctors;
S.LookupConstructors(DestRecordDecl, Ctors);
for (NamedDecl *D : Ctors) {
auto Info = getConstructorInfo(D);
if (!Info.Constructor)
continue;
Expand Down Expand Up @@ -5987,7 +5992,8 @@ static ExprResult CopyObject(Sema &S,
// C++11 [dcl.init]p16, second bullet for class types, this initialization
// is direct-initialization.
OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);
DeclContext::lookup_result Ctors = S.LookupConstructors(Class);
llvm::SmallVector<NamedDecl*, 4> Ctors;
S.LookupConstructors(Class, Ctors);

OverloadCandidateSet::iterator Best;
switch (ResolveConstructorOverload(
Expand Down Expand Up @@ -6129,8 +6135,8 @@ static void CheckCXX98CompatAccessibleCopy(Sema &S,

// Find constructors which would have been considered.
OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);
DeclContext::lookup_result Ctors =
S.LookupConstructors(cast<CXXRecordDecl>(Record->getDecl()));
llvm::SmallVector<NamedDecl*, 4> Ctors;
S.LookupConstructors(cast<CXXRecordDecl>(Record->getDecl()), Ctors);

// Perform overload resolution.
OverloadCandidateSet::iterator Best;
Expand Down
13 changes: 10 additions & 3 deletions interpreter/llvm/src/tools/clang/lib/Sema/SemaLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3214,7 +3214,8 @@ CXXConstructorDecl *Sema::LookupMovingConstructor(CXXRecordDecl *Class,
}

/// Look up the constructors for the given class.
DeclContext::lookup_result Sema::LookupConstructors(CXXRecordDecl *Class) {
void Sema::LookupConstructors(CXXRecordDecl *Class,
llvm::SmallVectorImpl<NamedDecl*> &Constructors) {
// If the implicit constructors have not yet been declared, do so now.
if (CanDeclareSpecialMemberFunction(Class)) {
if (Class->needsImplicitDefaultConstructor())
Expand All @@ -3227,7 +3228,10 @@ DeclContext::lookup_result Sema::LookupConstructors(CXXRecordDecl *Class) {

CanQualType T = Context.getCanonicalType(Context.getTypeDeclType(Class));
DeclarationName Name = Context.DeclarationNames.getCXXConstructorName(T);
return Class->lookup(Name);
// Working directly on R might trigger a deserialization, invalidating R if
// the underlying data structure needs to reallocate the storage.
DeclContext::lookup_result R = Class->lookup(Name);
Constructors.append(R.begin(), R.end());
}

/// Look up the copying assignment operator for the given class.
Expand Down Expand Up @@ -3462,7 +3466,10 @@ void Sema::ArgumentDependentLookup(DeclarationName Name, SourceLocation Loc,
// namespaces even if they are not visible during an ordinary
// lookup (11.4).
DeclContext::lookup_result R = NS->lookup(Name);
for (auto *D : R) {
// The loop might trigger a deserialization, invalidating R if the
// underlying data structure needs to reallocate the storage.
llvm::SmallVector<NamedDecl*, 8> RCopy(R.begin(), R.end());
for (auto *D : RCopy) {
auto *Underlying = D;
if (auto *USD = dyn_cast<UsingShadowDecl>(D))
Underlying = USD->getTargetDecl();
Expand Down
8 changes: 6 additions & 2 deletions interpreter/llvm/src/tools/clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3230,7 +3230,9 @@ IsInitializerListConstructorConversion(Sema &S, Expr *From, QualType ToType,
OverloadCandidateSet &CandidateSet,
bool AllowExplicit) {
CandidateSet.clear(OverloadCandidateSet::CSK_InitByUserDefinedConversion);
for (auto *D : S.LookupConstructors(To)) {
llvm::SmallVector<NamedDecl*, 4> Ctors;
S.LookupConstructors(To, Ctors);
for (auto *D : Ctors) {
auto Info = getConstructorInfo(D);
if (!Info)
continue;
Expand Down Expand Up @@ -3353,7 +3355,9 @@ IsUserDefinedConversion(Sema &S, Expr *From, QualType ToType,
ListInitializing = true;
}

for (auto *D : S.LookupConstructors(ToRecordDecl)) {
llvm::SmallVector<NamedDecl*, 4> Ctors;
S.LookupConstructors(ToRecordDecl, Ctors);
for (auto *D : Ctors) {
auto Info = getConstructorInfo(D);
if (!Info)
continue;
Expand Down
4 changes: 3 additions & 1 deletion interpreter/llvm/src/tools/clang/lib/Sema/SemaTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2158,7 +2158,9 @@ void Sema::DeclareImplicitDeductionGuides(TemplateDecl *Template,
// for which some class template parameter without a default argument never
// appears in a deduced context).
bool AddedAny = false;
for (NamedDecl *D : LookupConstructors(Transform.Primary)) {
llvm::SmallVector<NamedDecl*, 4> Ctors;
LookupConstructors(Transform.Primary, Ctors);
for (NamedDecl *D : Ctors) {
D = D->getUnderlyingDecl();
if (D->isInvalidDecl() || D->isImplicit())
continue;
Expand Down

0 comments on commit 6494f1f

Please sign in to comment.