[clang] bbb8a0d - [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision
Shafik Yaghmour via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 8 09:40:29 PST 2023
Author: Shafik Yaghmour
Date: 2023-12-08T09:38:59-08:00
New Revision: bbb8a0df7367068e1cf2fc54edd376beb976b430
URL: https://github.com/llvm/llvm-project/commit/bbb8a0df7367068e1cf2fc54edd376beb976b430
DIFF: https://github.com/llvm/llvm-project/commit/bbb8a0df7367068e1cf2fc54edd376beb976b430.diff
LOG: [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:
https://github.com/llvm/llvm-project/issues/39319
https://github.com/llvm/llvm-project/issues/60182
https://github.com/llvm/llvm-project/issues/62157
https://github.com/llvm/llvm-project/issues/64885
https://github.com/llvm/llvm-project/issues/65568
Differential Revision: https://reviews.llvm.org/D148474
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaCXX/cxx1z-copy-omission.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 626585c146914c..28f9393e28437d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 042ca61abe44d5..5ca6b232df66a5 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -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());
@@ -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())) {
@@ -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
@@ -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.
@@ -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:
@@ -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(
@@ -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;
@@ -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)
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 4ae027bd1bb673..df6b40999e645c 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -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)
diff --git a/clang/test/SemaCXX/cxx1z-copy-omission.cpp b/clang/test/SemaCXX/cxx1z-copy-omission.cpp
index a850cf6143cd48..f46a17af833867 100644
--- a/clang/test/SemaCXX/cxx1z-copy-omission.cpp
+++ b/clang/test/SemaCXX/cxx1z-copy-omission.cpp
@@ -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}}
+}
More information about the cfe-commits
mailing list