Skip to content

Commit

Permalink
[Clang] Fix ResolveConstructorOverload to not select a conversion fun…
Browse files Browse the repository at this point in the history
…ction 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
  • Loading branch information
shafik committed Dec 8, 2023
1 parent 5507f70 commit bbb8a0d
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 26 deletions.
6 changes: 6 additions & 0 deletions clang/docs/ReleaseNotes.rst
Expand Up @@ -669,6 +669,12 @@ Bug Fixes in This Version
Fixes (`#64467 <https://github.com/llvm/llvm-project/issues/64467>`_)
- Clang's ``-Wchar-subscripts`` no longer warns on chars whose values are known non-negative constants.
Fixes (`#18763 <https://github.com/llvm/llvm-project/issues/18763>`_)
- Fix crash due to incorrectly allowing conversion functions in copy elision.
Fixes (`#39319 <https://github.com/llvm/llvm-project/issues/39319>`_) and
(`#60182 <https://github.com/llvm/llvm-project/issues/60182>`_) and
(`#62157 <https://github.com/llvm/llvm-project/issues/62157>`_) and
(`#64885 <https://github.com/llvm/llvm-project/issues/64885>`_) and
(`#65568 <https://github.com/llvm/llvm-project/issues/65568>`_)

Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
51 changes: 25 additions & 26 deletions clang/lib/Sema/SemaInit.cpp
Expand Up @@ -4085,16 +4085,13 @@ static bool hasCopyOrMoveCtorParam(ASTContext &Ctx,
return Ctx.hasSameUnqualifiedType(ParmT, ClassT);
}

static OverloadingResult
ResolveConstructorOverload(Sema &S, SourceLocation DeclLoc,
MultiExprArg Args,
OverloadCandidateSet &CandidateSet,
QualType DestType,
DeclContext::lookup_result Ctors,
OverloadCandidateSet::iterator &Best,
bool CopyInitializing, bool AllowExplicit,
bool OnlyListConstructors, bool IsListInit,
bool SecondStepOfCopyInit = false) {
static OverloadingResult ResolveConstructorOverload(
Sema &S, SourceLocation DeclLoc, MultiExprArg Args,
OverloadCandidateSet &CandidateSet, QualType DestType,
DeclContext::lookup_result Ctors, OverloadCandidateSet::iterator &Best,
bool CopyInitializing, bool AllowExplicit, bool OnlyListConstructors,
bool IsListInit, bool RequireActualConstructor,
bool SecondStepOfCopyInit = false) {
CandidateSet.clear(OverloadCandidateSet::CSK_InitByConstructor);
CandidateSet.setDestAS(DestType.getQualifiers().getAddressSpace());

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

bool RequireActualConstructor =
!(Entity.getKind() != InitializedEntity::EK_Base &&
Entity.getKind() != InitializedEntity::EK_Delegating &&
Entity.getKind() !=
InitializedEntity::EK_LambdaToBlockConversionBlockElement);

// C++17 [dcl.init]p17:
// - If the initializer expression is a prvalue and the cv-unqualified
// version of the source type is the same class as the class of the
Expand All @@ -4234,11 +4237,7 @@ static void TryConstructorInitialization(Sema &S,
// class or delegating to another constructor from a mem-initializer.
// ObjC++: Lambda captured by the block in the lambda to block conversion
// should avoid copy elision.
if (S.getLangOpts().CPlusPlus17 &&
Entity.getKind() != InitializedEntity::EK_Base &&
Entity.getKind() != InitializedEntity::EK_Delegating &&
Entity.getKind() !=
InitializedEntity::EK_LambdaToBlockConversionBlockElement &&
if (S.getLangOpts().CPlusPlus17 && !RequireActualConstructor &&
UnwrappedArgs.size() == 1 && UnwrappedArgs[0]->isPRValue() &&
S.Context.hasSameUnqualifiedType(UnwrappedArgs[0]->getType(), DestType)) {
// Convert qualifications if necessary.
Expand Down Expand Up @@ -4286,11 +4285,10 @@ static void TryConstructorInitialization(Sema &S,
// If the initializer list has no elements and T has a default constructor,
// the first phase is omitted.
if (!(UnwrappedArgs.empty() && S.LookupDefaultConstructor(DestRecordDecl)))
Result = ResolveConstructorOverload(S, Kind.getLocation(), Args,
CandidateSet, DestType, Ctors, Best,
CopyInitialization, AllowExplicit,
/*OnlyListConstructors=*/true,
IsListInit);
Result = ResolveConstructorOverload(
S, Kind.getLocation(), Args, CandidateSet, DestType, Ctors, Best,
CopyInitialization, AllowExplicit,
/*OnlyListConstructors=*/true, IsListInit, RequireActualConstructor);
}

// C++11 [over.match.list]p1:
Expand All @@ -4300,11 +4298,10 @@ static void TryConstructorInitialization(Sema &S,
// elements of the initializer list.
if (Result == OR_No_Viable_Function) {
AsInitializerList = false;
Result = ResolveConstructorOverload(S, Kind.getLocation(), UnwrappedArgs,
CandidateSet, DestType, Ctors, Best,
CopyInitialization, AllowExplicit,
/*OnlyListConstructors=*/false,
IsListInit);
Result = ResolveConstructorOverload(
S, Kind.getLocation(), UnwrappedArgs, CandidateSet, DestType, Ctors,
Best, CopyInitialization, AllowExplicit,
/*OnlyListConstructors=*/false, IsListInit, RequireActualConstructor);
}
if (Result) {
Sequence.SetOverloadFailure(
Expand Down Expand Up @@ -6778,6 +6775,7 @@ static ExprResult CopyObject(Sema &S,
S, Loc, CurInitExpr, CandidateSet, T, Ctors, Best,
/*CopyInitializing=*/false, /*AllowExplicit=*/true,
/*OnlyListConstructors=*/false, /*IsListInit=*/false,
/*RequireActualConstructor=*/false,
/*SecondStepOfCopyInit=*/true)) {
case OR_Success:
break;
Expand Down Expand Up @@ -6920,6 +6918,7 @@ static void CheckCXX98CompatAccessibleCopy(Sema &S,
S, Loc, CurInitExpr, CandidateSet, CurInitExpr->getType(), Ctors, Best,
/*CopyInitializing=*/false, /*AllowExplicit=*/true,
/*OnlyListConstructors=*/false, /*IsListInit=*/false,
/*RequireActualConstructor=*/false,
/*SecondStepOfCopyInit=*/true);

PartialDiagnostic Diag = S.PDiag(diag::warn_cxx98_compat_temp_copy)
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Sema/SemaTemplateInstantiate.cpp
Expand Up @@ -810,6 +810,10 @@ void Sema::PrintInstantiationStack() {
Diags.Report(Active->PointOfInstantiation,
diag::note_template_nsdmi_here)
<< FD << Active->InstantiationRange;
} else if (ClassTemplateDecl *CTD = dyn_cast<ClassTemplateDecl>(D)) {
Diags.Report(Active->PointOfInstantiation,
diag::note_template_class_instantiation_here)
<< CTD << Active->InstantiationRange;
} else {
Diags.Report(Active->PointOfInstantiation,
diag::note_template_type_alias_instantiation_here)
Expand Down
27 changes: 27 additions & 0 deletions clang/test/SemaCXX/cxx1z-copy-omission.cpp
Expand Up @@ -171,3 +171,30 @@ namespace CtorTemplateBeatsNonTemplateConversionFn {
Foo f(Derived d) { return d; } // expected-error {{invokes a deleted function}}
Foo g(Derived d) { return Foo(d); } // ok, calls constructor
}

// Make sure we don't consider conversion functions for guaranteed copy elision
namespace GH39319 {
struct A {
A();
A(const A&) = delete; // expected-note {{'A' has been explicitly marked deleted here}}
};
struct B {
operator A();
} C;
A::A() : A(C) {} // expected-error {{call to deleted constructor of}}

struct A2 {
struct B2 {
operator A2();
};
A2() : A2(B2()) {} // expected-error {{call to deleted constructor of}}
A2(const A2&) = delete; // expected-note {{'A2' has been explicitly marked deleted here}}
};

template<typename A3>
class B3 : A3 {
template<bool = C3<B3>()> // expected-warning 2{{use of function template name with no prior declaration in function call with explicit}}
B3();
}; B3(); // expected-error {{deduction guide declaration without trailing return type}} \
// expected-note {{while building implicit deduction guide first needed here}}
}

0 comments on commit bbb8a0d

Please sign in to comment.