Skip to content

Commit bbb8a0d

Browse files
committed
[Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision
ResolveConstructorOverload needs to check properly if we are going to use copy elision we can't use a conversion function. This fixes: #39319 #60182 #62157 #64885 #65568 Differential Revision: https://reviews.llvm.org/D148474
1 parent 5507f70 commit bbb8a0d

File tree

4 files changed

+62
-26
lines changed

4 files changed

+62
-26
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,12 @@ Bug Fixes in This Version
669669
Fixes (`#64467 <https://github.com/llvm/llvm-project/issues/64467>`_)
670670
- Clang's ``-Wchar-subscripts`` no longer warns on chars whose values are known non-negative constants.
671671
Fixes (`#18763 <https://github.com/llvm/llvm-project/issues/18763>`_)
672+
- Fix crash due to incorrectly allowing conversion functions in copy elision.
673+
Fixes (`#39319 <https://github.com/llvm/llvm-project/issues/39319>`_) and
674+
(`#60182 <https://github.com/llvm/llvm-project/issues/60182>`_) and
675+
(`#62157 <https://github.com/llvm/llvm-project/issues/62157>`_) and
676+
(`#64885 <https://github.com/llvm/llvm-project/issues/64885>`_) and
677+
(`#65568 <https://github.com/llvm/llvm-project/issues/65568>`_)
672678

673679
Bug Fixes to Compiler Builtins
674680
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/Sema/SemaInit.cpp

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4085,16 +4085,13 @@ static bool hasCopyOrMoveCtorParam(ASTContext &Ctx,
40854085
return Ctx.hasSameUnqualifiedType(ParmT, ClassT);
40864086
}
40874087

4088-
static OverloadingResult
4089-
ResolveConstructorOverload(Sema &S, SourceLocation DeclLoc,
4090-
MultiExprArg Args,
4091-
OverloadCandidateSet &CandidateSet,
4092-
QualType DestType,
4093-
DeclContext::lookup_result Ctors,
4094-
OverloadCandidateSet::iterator &Best,
4095-
bool CopyInitializing, bool AllowExplicit,
4096-
bool OnlyListConstructors, bool IsListInit,
4097-
bool SecondStepOfCopyInit = false) {
4088+
static OverloadingResult ResolveConstructorOverload(
4089+
Sema &S, SourceLocation DeclLoc, MultiExprArg Args,
4090+
OverloadCandidateSet &CandidateSet, QualType DestType,
4091+
DeclContext::lookup_result Ctors, OverloadCandidateSet::iterator &Best,
4092+
bool CopyInitializing, bool AllowExplicit, bool OnlyListConstructors,
4093+
bool IsListInit, bool RequireActualConstructor,
4094+
bool SecondStepOfCopyInit = false) {
40984095
CandidateSet.clear(OverloadCandidateSet::CSK_InitByConstructor);
40994096
CandidateSet.setDestAS(DestType.getQualifiers().getAddressSpace());
41004097

@@ -4157,7 +4154,7 @@ ResolveConstructorOverload(Sema &S, SourceLocation DeclLoc,
41574154
// Note: SecondStepOfCopyInit is only ever true in this case when
41584155
// evaluating whether to produce a C++98 compatibility warning.
41594156
if (S.getLangOpts().CPlusPlus17 && Args.size() == 1 &&
4160-
!SecondStepOfCopyInit) {
4157+
!RequireActualConstructor && !SecondStepOfCopyInit) {
41614158
Expr *Initializer = Args[0];
41624159
auto *SourceRD = Initializer->getType()->getAsCXXRecordDecl();
41634160
if (SourceRD && S.isCompleteType(DeclLoc, Initializer->getType())) {
@@ -4225,6 +4222,12 @@ static void TryConstructorInitialization(Sema &S,
42254222
return;
42264223
}
42274224

4225+
bool RequireActualConstructor =
4226+
!(Entity.getKind() != InitializedEntity::EK_Base &&
4227+
Entity.getKind() != InitializedEntity::EK_Delegating &&
4228+
Entity.getKind() !=
4229+
InitializedEntity::EK_LambdaToBlockConversionBlockElement);
4230+
42284231
// C++17 [dcl.init]p17:
42294232
// - If the initializer expression is a prvalue and the cv-unqualified
42304233
// version of the source type is the same class as the class of the
@@ -4234,11 +4237,7 @@ static void TryConstructorInitialization(Sema &S,
42344237
// class or delegating to another constructor from a mem-initializer.
42354238
// ObjC++: Lambda captured by the block in the lambda to block conversion
42364239
// should avoid copy elision.
4237-
if (S.getLangOpts().CPlusPlus17 &&
4238-
Entity.getKind() != InitializedEntity::EK_Base &&
4239-
Entity.getKind() != InitializedEntity::EK_Delegating &&
4240-
Entity.getKind() !=
4241-
InitializedEntity::EK_LambdaToBlockConversionBlockElement &&
4240+
if (S.getLangOpts().CPlusPlus17 && !RequireActualConstructor &&
42424241
UnwrappedArgs.size() == 1 && UnwrappedArgs[0]->isPRValue() &&
42434242
S.Context.hasSameUnqualifiedType(UnwrappedArgs[0]->getType(), DestType)) {
42444243
// Convert qualifications if necessary.
@@ -4286,11 +4285,10 @@ static void TryConstructorInitialization(Sema &S,
42864285
// If the initializer list has no elements and T has a default constructor,
42874286
// the first phase is omitted.
42884287
if (!(UnwrappedArgs.empty() && S.LookupDefaultConstructor(DestRecordDecl)))
4289-
Result = ResolveConstructorOverload(S, Kind.getLocation(), Args,
4290-
CandidateSet, DestType, Ctors, Best,
4291-
CopyInitialization, AllowExplicit,
4292-
/*OnlyListConstructors=*/true,
4293-
IsListInit);
4288+
Result = ResolveConstructorOverload(
4289+
S, Kind.getLocation(), Args, CandidateSet, DestType, Ctors, Best,
4290+
CopyInitialization, AllowExplicit,
4291+
/*OnlyListConstructors=*/true, IsListInit, RequireActualConstructor);
42944292
}
42954293

42964294
// C++11 [over.match.list]p1:
@@ -4300,11 +4298,10 @@ static void TryConstructorInitialization(Sema &S,
43004298
// elements of the initializer list.
43014299
if (Result == OR_No_Viable_Function) {
43024300
AsInitializerList = false;
4303-
Result = ResolveConstructorOverload(S, Kind.getLocation(), UnwrappedArgs,
4304-
CandidateSet, DestType, Ctors, Best,
4305-
CopyInitialization, AllowExplicit,
4306-
/*OnlyListConstructors=*/false,
4307-
IsListInit);
4301+
Result = ResolveConstructorOverload(
4302+
S, Kind.getLocation(), UnwrappedArgs, CandidateSet, DestType, Ctors,
4303+
Best, CopyInitialization, AllowExplicit,
4304+
/*OnlyListConstructors=*/false, IsListInit, RequireActualConstructor);
43084305
}
43094306
if (Result) {
43104307
Sequence.SetOverloadFailure(
@@ -6778,6 +6775,7 @@ static ExprResult CopyObject(Sema &S,
67786775
S, Loc, CurInitExpr, CandidateSet, T, Ctors, Best,
67796776
/*CopyInitializing=*/false, /*AllowExplicit=*/true,
67806777
/*OnlyListConstructors=*/false, /*IsListInit=*/false,
6778+
/*RequireActualConstructor=*/false,
67816779
/*SecondStepOfCopyInit=*/true)) {
67826780
case OR_Success:
67836781
break;
@@ -6920,6 +6918,7 @@ static void CheckCXX98CompatAccessibleCopy(Sema &S,
69206918
S, Loc, CurInitExpr, CandidateSet, CurInitExpr->getType(), Ctors, Best,
69216919
/*CopyInitializing=*/false, /*AllowExplicit=*/true,
69226920
/*OnlyListConstructors=*/false, /*IsListInit=*/false,
6921+
/*RequireActualConstructor=*/false,
69236922
/*SecondStepOfCopyInit=*/true);
69246923

69256924
PartialDiagnostic Diag = S.PDiag(diag::warn_cxx98_compat_temp_copy)

clang/lib/Sema/SemaTemplateInstantiate.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,10 @@ void Sema::PrintInstantiationStack() {
810810
Diags.Report(Active->PointOfInstantiation,
811811
diag::note_template_nsdmi_here)
812812
<< FD << Active->InstantiationRange;
813+
} else if (ClassTemplateDecl *CTD = dyn_cast<ClassTemplateDecl>(D)) {
814+
Diags.Report(Active->PointOfInstantiation,
815+
diag::note_template_class_instantiation_here)
816+
<< CTD << Active->InstantiationRange;
813817
} else {
814818
Diags.Report(Active->PointOfInstantiation,
815819
diag::note_template_type_alias_instantiation_here)

clang/test/SemaCXX/cxx1z-copy-omission.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,3 +171,30 @@ namespace CtorTemplateBeatsNonTemplateConversionFn {
171171
Foo f(Derived d) { return d; } // expected-error {{invokes a deleted function}}
172172
Foo g(Derived d) { return Foo(d); } // ok, calls constructor
173173
}
174+
175+
// Make sure we don't consider conversion functions for guaranteed copy elision
176+
namespace GH39319 {
177+
struct A {
178+
A();
179+
A(const A&) = delete; // expected-note {{'A' has been explicitly marked deleted here}}
180+
};
181+
struct B {
182+
operator A();
183+
} C;
184+
A::A() : A(C) {} // expected-error {{call to deleted constructor of}}
185+
186+
struct A2 {
187+
struct B2 {
188+
operator A2();
189+
};
190+
A2() : A2(B2()) {} // expected-error {{call to deleted constructor of}}
191+
A2(const A2&) = delete; // expected-note {{'A2' has been explicitly marked deleted here}}
192+
};
193+
194+
template<typename A3>
195+
class B3 : A3 {
196+
template<bool = C3<B3>()> // expected-warning 2{{use of function template name with no prior declaration in function call with explicit}}
197+
B3();
198+
}; B3(); // expected-error {{deduction guide declaration without trailing return type}} \
199+
// expected-note {{while building implicit deduction guide first needed here}}
200+
}

0 commit comments

Comments
 (0)