From e43b3d1f5e05c6e5e07cff054df193cf0d0c6583 Mon Sep 17 00:00:00 2001 From: Yang Fan Date: Mon, 4 Jan 2021 17:21:19 +0800 Subject: [PATCH] Revert "[Sema] Fix deleted function problem in implicitly movable test" This reverts commit 89b0972a --- clang/lib/Sema/SemaInit.cpp | 67 ++++++------------- clang/lib/Sema/SemaStmt.cpp | 36 ++++------ .../class.init/class.copy.elision/p3.cpp | 50 -------------- 3 files changed, 33 insertions(+), 120 deletions(-) delete mode 100644 clang/test/CXX/class/class.init/class.copy.elision/p3.cpp diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 4a965c60c74e..6d2e6094e79c 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -4119,9 +4119,7 @@ static void TryConstructorInitialization(Sema &S, InitializationSequence::FK_ListConstructorOverloadFailed : InitializationSequence::FK_ConstructorOverloadFailed, Result); - - if (Result != OR_Deleted) - return; + return; } bool HadMultipleCandidates = (CandidateSet.size() > 1); @@ -4142,45 +4140,31 @@ 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(Best->Function); - if (Result != OR_Deleted) { // TODO: Support for more than one failure. - // 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); + if (Kind.getKind() == InitializationKind::IK_Default && + Entity.getType().isConstQualified()) { + if (!CtorDecl->getParent()->allowConstDefaultInit()) { + if (!maybeRecoverWithZeroInitialization(S, Sequence, Entity)) + Sequence.SetFailed(InitializationSequence::FK_DefaultInitOfConst); return; } } - // [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) + // 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; + } // Add the constructor initialization step. Any cv-qualification conversion is // subsumed by the initialization. @@ -5276,16 +5260,7 @@ static void TryUserDefinedConversion(Sema &S, Sequence.SetOverloadFailure( 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; + return; } FunctionDecl *Function = Best->Function; diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 3318fda07175..027262ca8ec9 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -3118,12 +3118,7 @@ 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. -/// -/// \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, +static void TryMoveInitialization(Sema &S, const InitializedEntity &Entity, const VarDecl *NRVOCandidate, QualType ResultType, Expr *&Value, bool ConvertingConstructorsOnly, @@ -3138,10 +3133,8 @@ static bool TryMoveInitialization(Sema &S, const InitializedEntity &Entity, InitializationSequence Seq(S, Entity, Kind, InitExpr); - bool NeedSecondOverloadResolution = true; - if (!Seq && Seq.getFailedOverloadResult() != OR_Deleted) { - return NeedSecondOverloadResolution; - } + if (!Seq) + return; for (const InitializationSequence::Step &Step : Seq.steps()) { if (Step.Kind != InitializationSequence::SK_ConstructorInitialization && @@ -3184,7 +3177,6 @@ static bool TryMoveInitialization(Sema &S, const InitializedEntity &Entity, } } - NeedSecondOverloadResolution = false; // Promote "AsRvalue" to the heap, since we now need this // expression node to persist. Value = @@ -3195,8 +3187,6 @@ static bool 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 @@ -3221,7 +3211,6 @@ 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; @@ -3238,15 +3227,14 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity, } if (NRVOCandidate) { - NeedSecondOverloadResolution = TryMoveInitialization( - *this, Entity, NRVOCandidate, ResultType, Value, true, Res); + TryMoveInitialization(*this, Entity, NRVOCandidate, ResultType, Value, + true, Res); } - if (!NeedSecondOverloadResolution && AffectedByCWG1579) { + if (!Res.isInvalid() && AffectedByCWG1579) { QualType QT = NRVOCandidate->getType(); - if (QT.getNonReferenceType() - .getUnqualifiedType() - .isTriviallyCopyableType(Context)) { + if (QT.getNonReferenceType().getUnqualifiedType().isTriviallyCopyableType( + Context)) { // Adding 'std::move' around a trivially copyable variable is probably // pointless. Don't suggest it. } else { @@ -3260,12 +3248,12 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity, Str += NRVOCandidate->getDeclName().getAsString(); Str += ")"; Diag(Value->getExprLoc(), diag::warn_return_std_move_in_cxx11) - << Value->getSourceRange() - << NRVOCandidate->getDeclName() << ResultType << QT; + << Value->getSourceRange() << NRVOCandidate->getDeclName() + << ResultType << QT; Diag(Value->getExprLoc(), diag::note_add_std_move_in_cxx11) << FixItHint::CreateReplacement(Value->getSourceRange(), Str); } - } else if (NeedSecondOverloadResolution && + } else if (Res.isInvalid() && !getDiagnostics().isIgnored(diag::warn_return_std_move, Value->getExprLoc())) { const VarDecl *FakeNRVOCandidate = @@ -3306,7 +3294,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 (NeedSecondOverloadResolution) + if (Res.isInvalid()) 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 deleted file mode 100644 index b3427243bd61..000000000000 --- a/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp +++ /dev/null @@ -1,50 +0,0 @@ -// 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