[clang] 74f93bc - [Sema] Fix deleted function problem in implicitly movable test
Yang Fan via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 5 18:05:56 PST 2021
Author: Yang Fan
Date: 2021-01-06T10:05:40+08:00
New Revision: 74f93bc373d089e757bb65cf8b30b63a4eae8b69
URL: https://github.com/llvm/llvm-project/commit/74f93bc373d089e757bb65cf8b30b63a4eae8b69
DIFF: https://github.com/llvm/llvm-project/commit/74f93bc373d089e757bb65cf8b30b63a4eae8b69.diff
LOG: [Sema] Fix deleted function problem in implicitly movable test
In implicitly movable test, a two-stage overload resolution is performed.
If the first overload resolution selects a deleted function, Clang directly
performs the second overload resolution, without checking whether the
deleted function matches the additional criteria.
This patch fixes the above problem.
Reviewed By: Quuxplusone
Differential Revision: https://reviews.llvm.org/D92936
Added:
clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
Modified:
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaCXX/warn-return-std-move.cpp
Removed:
################################################################################
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 6d2e6094e79c..b5f31bf403d4 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -4115,11 +4115,13 @@ static void TryConstructorInitialization(Sema &S,
IsListInit);
}
if (Result) {
- Sequence.SetOverloadFailure(IsListInit ?
- InitializationSequence::FK_ListConstructorOverloadFailed :
- InitializationSequence::FK_ConstructorOverloadFailed,
- Result);
- return;
+ Sequence.SetOverloadFailure(
+ IsListInit ? InitializationSequence::FK_ListConstructorOverloadFailed
+ : InitializationSequence::FK_ConstructorOverloadFailed,
+ Result);
+
+ if (Result != OR_Deleted)
+ return;
}
bool HadMultipleCandidates = (CandidateSet.size() > 1);
@@ -4140,31 +4142,45 @@ static void TryConstructorInitialization(Sema &S,
return;
}
- // C++11 [dcl.init]p6:
- // If a program calls for the default initialization of an object
- // of a const-qualified type T, T shall be a class type with a
- // user-provided default constructor.
- // C++ core issue 253 proposal:
- // If the implicit default constructor initializes all subobjects, no
- // initializer should be required.
- // The 253 proposal is for example needed to process libstdc++ headers in 5.x.
CXXConstructorDecl *CtorDecl = cast<CXXConstructorDecl>(Best->Function);
- if (Kind.getKind() == InitializationKind::IK_Default &&
- Entity.getType().isConstQualified()) {
- if (!CtorDecl->getParent()->allowConstDefaultInit()) {
- if (!maybeRecoverWithZeroInitialization(S, Sequence, Entity))
- Sequence.SetFailed(InitializationSequence::FK_DefaultInitOfConst);
+ if (Result != OR_Deleted) {
+ // C++11 [dcl.init]p6:
+ // If a program calls for the default initialization of an object
+ // of a const-qualified type T, T shall be a class type with a
+ // user-provided default constructor.
+ // C++ core issue 253 proposal:
+ // If the implicit default constructor initializes all subobjects, no
+ // initializer should be required.
+ // The 253 proposal is for example needed to process libstdc++ headers
+ // in 5.x.
+ if (Kind.getKind() == InitializationKind::IK_Default &&
+ Entity.getType().isConstQualified()) {
+ if (!CtorDecl->getParent()->allowConstDefaultInit()) {
+ if (!maybeRecoverWithZeroInitialization(S, Sequence, Entity))
+ Sequence.SetFailed(InitializationSequence::FK_DefaultInitOfConst);
+ return;
+ }
+ }
+
+ // C++11 [over.match.list]p1:
+ // In copy-list-initialization, if an explicit constructor is chosen, the
+ // initializer is ill-formed.
+ if (IsListInit && !Kind.AllowExplicit() && CtorDecl->isExplicit()) {
+ Sequence.SetFailed(InitializationSequence::FK_ExplicitConstructor);
return;
}
}
- // C++11 [over.match.list]p1:
- // In copy-list-initialization, if an explicit constructor is chosen, the
- // initializer is ill-formed.
- if (IsListInit && !Kind.AllowExplicit() && CtorDecl->isExplicit()) {
- Sequence.SetFailed(InitializationSequence::FK_ExplicitConstructor);
+ // [class.copy.elision]p3:
+ // In some copy-initialization contexts, a two-stage overload resolution
+ // is performed.
+ // If the first overload resolution selects a deleted function, we also
+ // need the initialization sequence to decide whether to perform the second
+ // overload resolution.
+ // For deleted functions in other contexts, there is no need to get the
+ // initialization sequence.
+ if (Result == OR_Deleted && Kind.getKind() != InitializationKind::IK_Copy)
return;
- }
// Add the constructor initialization step. Any cv-qualification conversion is
// subsumed by the initialization.
@@ -5258,9 +5274,17 @@ static void TryUserDefinedConversion(Sema &S,
if (OverloadingResult Result
= CandidateSet.BestViableFunction(S, DeclLoc, Best)) {
Sequence.SetOverloadFailure(
- InitializationSequence::FK_UserConversionOverloadFailed,
- Result);
- return;
+ InitializationSequence::FK_UserConversionOverloadFailed, Result);
+
+ // [class.copy.elision]p3:
+ // In some copy-initialization contexts, a two-stage overload resolution
+ // is performed.
+ // If the first overload resolution selects a deleted function, we also
+ // need the initialization sequence to decide whether to perform the second
+ // overload resolution.
+ if (!(Result == OR_Deleted &&
+ Kind.getKind() == InitializationKind::IK_Copy))
+ return;
}
FunctionDecl *Function = Best->Function;
@@ -5564,13 +5588,11 @@ static bool TryOCLZeroOpaqueTypeInitialization(Sema &S,
return false;
}
-InitializationSequence::InitializationSequence(Sema &S,
- const InitializedEntity &Entity,
- const InitializationKind &Kind,
- MultiExprArg Args,
- bool TopLevelOfInitList,
- bool TreatUnavailableAsInvalid)
- : FailedCandidateSet(Kind.getLocation(), OverloadCandidateSet::CSK_Normal) {
+InitializationSequence::InitializationSequence(
+ Sema &S, const InitializedEntity &Entity, const InitializationKind &Kind,
+ MultiExprArg Args, bool TopLevelOfInitList, bool TreatUnavailableAsInvalid)
+ : FailedOverloadResult(OR_Success),
+ FailedCandidateSet(Kind.getLocation(), OverloadCandidateSet::CSK_Normal) {
InitializeFrom(S, Entity, Kind, Args, TopLevelOfInitList,
TreatUnavailableAsInvalid);
}
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 027262ca8ec9..a47fdf625bba 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3118,11 +3118,16 @@ bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
/// If move-initialization is not possible, such that we must fall back to
/// treating the operand as an lvalue, we will leave Res in its original
/// invalid state.
-static void TryMoveInitialization(Sema &S, const InitializedEntity &Entity,
+///
+/// \returns Whether we need to do the second overload resolution. If the first
+/// overload resolution fails, or if the first overload resolution succeeds but
+/// the selected constructor/operator doesn't match the additional criteria, we
+/// need to do the second overload resolution.
+static bool TryMoveInitialization(Sema &S, const InitializedEntity &Entity,
const VarDecl *NRVOCandidate,
QualType ResultType, Expr *&Value,
bool ConvertingConstructorsOnly,
- ExprResult &Res) {
+ bool IsDiagnosticsCheck, ExprResult &Res) {
ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
CK_NoOp, Value, VK_XValue, FPOptionsOverride());
@@ -3133,8 +3138,11 @@ static void TryMoveInitialization(Sema &S, const InitializedEntity &Entity,
InitializationSequence Seq(S, Entity, Kind, InitExpr);
- if (!Seq)
- return;
+ bool NeedSecondOverloadResolution = true;
+ if (!Seq &&
+ (IsDiagnosticsCheck || Seq.getFailedOverloadResult() != OR_Deleted)) {
+ return NeedSecondOverloadResolution;
+ }
for (const InitializationSequence::Step &Step : Seq.steps()) {
if (Step.Kind != InitializationSequence::SK_ConstructorInitialization &&
@@ -3177,6 +3185,7 @@ static void TryMoveInitialization(Sema &S, const InitializedEntity &Entity,
}
}
+ NeedSecondOverloadResolution = false;
// Promote "AsRvalue" to the heap, since we now need this
// expression node to persist.
Value =
@@ -3187,6 +3196,8 @@ static void TryMoveInitialization(Sema &S, const InitializedEntity &Entity,
// using the constructor we found.
Res = Seq.Perform(S, Entity, Kind, Value);
}
+
+ return NeedSecondOverloadResolution;
}
/// Perform the initialization of a potentially-movable value, which
@@ -3211,6 +3222,7 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
// select the constructor for the copy is first performed as if the object
// were designated by an rvalue.
ExprResult Res = ExprError();
+ bool NeedSecondOverloadResolution = true;
if (AllowNRVO) {
bool AffectedByCWG1579 = false;
@@ -3227,11 +3239,11 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
}
if (NRVOCandidate) {
- TryMoveInitialization(*this, Entity, NRVOCandidate, ResultType, Value,
- true, Res);
+ NeedSecondOverloadResolution = TryMoveInitialization(
+ *this, Entity, NRVOCandidate, ResultType, Value, true, false, Res);
}
- if (!Res.isInvalid() && AffectedByCWG1579) {
+ if (!NeedSecondOverloadResolution && AffectedByCWG1579) {
QualType QT = NRVOCandidate->getType();
if (QT.getNonReferenceType().getUnqualifiedType().isTriviallyCopyableType(
Context)) {
@@ -3253,7 +3265,7 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
Diag(Value->getExprLoc(), diag::note_add_std_move_in_cxx11)
<< FixItHint::CreateReplacement(Value->getSourceRange(), Str);
}
- } else if (Res.isInvalid() &&
+ } else if (NeedSecondOverloadResolution &&
!getDiagnostics().isIgnored(diag::warn_return_std_move,
Value->getExprLoc())) {
const VarDecl *FakeNRVOCandidate =
@@ -3272,7 +3284,7 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
ExprResult FakeRes = ExprError();
Expr *FakeValue = Value;
TryMoveInitialization(*this, Entity, FakeNRVOCandidate, ResultType,
- FakeValue, false, FakeRes);
+ FakeValue, false, true, FakeRes);
if (!FakeRes.isInvalid()) {
bool IsThrow =
(Entity.getKind() == InitializedEntity::EK_Exception);
@@ -3294,7 +3306,7 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
// Either we didn't meet the criteria for treating an lvalue as an rvalue,
// above, or overload resolution failed. Either way, we need to try
// (again) now with the return value expression as written.
- if (Res.isInvalid())
+ if (NeedSecondOverloadResolution)
Res = PerformCopyInitialization(Entity, SourceLocation(), Value);
return Res;
diff --git a/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp b/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
new file mode 100644
index 000000000000..b3427243bd61
--- /dev/null
+++ b/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -verify=expected %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -verify=expected %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -verify=expected %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -verify=expected %s
+
+namespace test_delete_function {
+struct A1 {
+ A1();
+ A1(const A1 &);
+ A1(A1 &&) = delete; // expected-note {{'A1' has been explicitly marked deleted here}}
+};
+A1 test1() {
+ A1 a;
+ return a; // expected-error {{call to deleted constructor of 'test_delete_function::A1'}}
+}
+
+struct A2 {
+ A2();
+ A2(const A2 &);
+
+private:
+ A2(A2 &&); // expected-note {{declared private here}}
+};
+A2 test2() {
+ A2 a;
+ return a; // expected-error {{calling a private constructor of class 'test_delete_function::A2'}}
+}
+
+struct C {};
+
+struct B1 {
+ B1(C &);
+ B1(C &&) = delete; // expected-note {{'B1' has been explicitly marked deleted here}}
+};
+B1 test3() {
+ C c;
+ return c; // expected-error {{conversion function from 'test_delete_function::C' to 'test_delete_function::B1' invokes a deleted function}}
+}
+
+struct B2 {
+ B2(C &);
+
+private:
+ B2(C &&); // expected-note {{declared private here}}
+};
+B2 test4() {
+ C c;
+ return c; // expected-error {{calling a private constructor of class 'test_delete_function::B2'}}
+}
+} // namespace test_delete_function
diff --git a/clang/test/SemaCXX/warn-return-std-move.cpp b/clang/test/SemaCXX/warn-return-std-move.cpp
index 48542f467f21..9669c7d282d9 100644
--- a/clang/test/SemaCXX/warn-return-std-move.cpp
+++ b/clang/test/SemaCXX/warn-return-std-move.cpp
@@ -324,11 +324,29 @@ void test_throw1(Derived&& d) {
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:12}:"std::move(d)"
}
-void ok_throw1() { Derived d; throw d; }
+void ok_throw1() {
+ Derived d;
+ throw d;
+}
void ok_throw2(Derived d) { throw d; }
-void ok_throw3(Derived& d) { throw d; }
+void ok_throw3(Derived &d) { throw d; }
void ok_throw4(Derived d) { throw std::move(d); }
-void ok_throw5(Derived& d) { throw std::move(d); }
-void ok_throw6(Derived& d) { throw static_cast<Derived&&>(d); }
+void ok_throw5(Derived &d) { throw std::move(d); }
+void ok_throw6(Derived &d) { throw static_cast<Derived &&>(d); }
void ok_throw7(TriviallyCopyable d) { throw d; }
void ok_throw8(OnlyCopyable d) { throw d; }
+
+namespace test_delete {
+struct Base {
+ Base();
+ Base(Base &&) = delete;
+ Base(Base const &);
+};
+
+struct Derived : public Base {};
+
+Base test_ok() {
+ Derived d;
+ return d;
+}
+} // namespace test_delete
More information about the cfe-commits
mailing list