diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -493,7 +493,6 @@ def Padded : DiagGroup<"padded">; def PessimizingMove : DiagGroup<"pessimizing-move">; -def ReturnStdMoveInCXX11 : DiagGroup<"return-std-move-in-c++11">; def ReturnStdMove : DiagGroup<"return-std-move">; def PointerArith : DiagGroup<"pointer-arith">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6284,13 +6284,6 @@ InGroup, DefaultIgnore; def note_add_std_move : Note< "call 'std::move' explicitly to avoid copying">; -def warn_return_std_move_in_cxx11 : Warning< - "prior to the resolution of a defect report against ISO C++11, " - "local variable %0 would have been copied despite being returned by name, " - "due to its not matching the function return type%diff{ ($ vs $)|}1,2">, - InGroup, DefaultIgnore; -def note_add_std_move_in_cxx11 : Note< - "call 'std::move' explicitly to avoid copying on older compilers">; def warn_string_plus_int : Warning< "adding %0 to a string does not append to the string">, diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4623,10 +4623,15 @@ CES_AllowParameters = 1, CES_AllowDifferentTypes = 2, CES_AllowExceptionVariables = 4, - CES_FormerDefault = (CES_AllowParameters), - CES_Default = (CES_AllowParameters | CES_AllowDifferentTypes), - CES_AsIfByStdMove = (CES_AllowParameters | CES_AllowDifferentTypes | - CES_AllowExceptionVariables), + CES_AllowRValueReferenceType = 8, + CES_ImplicitlyMovableCXX11CXX14CXX17 = + (CES_AllowParameters | CES_AllowDifferentTypes), + CES_ImplicitlyMovableCXX20 = + (CES_AllowParameters | CES_AllowDifferentTypes | + CES_AllowExceptionVariables | CES_AllowRValueReferenceType), + CES_AsIfByStdMove = + (CES_AllowParameters | CES_AllowDifferentTypes | + CES_AllowExceptionVariables | CES_AllowRValueReferenceType), }; VarDecl *getCopyElisionCandidate(QualType ReturnType, Expr *E, diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -3078,6 +3078,13 @@ if (VD->getType().isVolatileQualified()) return false; + // C++20 [class.copy.elision]p3: + // ...rvalue reference to a non-volatile... + if (VD->getType()->isRValueReferenceType() && + (!(CESK & CES_AllowRValueReferenceType) || + VD->getType().getNonReferenceType().isVolatileQualified())) + return false; + if (CESK & CES_AllowDifferentTypes) return true; @@ -3093,13 +3100,13 @@ /// Try to perform the initialization of a potentially-movable value, /// which is the operand to a return or throw statement. /// -/// This routine implements C++14 [class.copy]p32, which attempts to treat -/// returned lvalues as rvalues in certain cases (to prefer move construction), -/// then falls back to treating them as lvalues if that failed. +/// This routine implements C++20 [class.copy.elision]p3, which attempts to +/// treat returned lvalues as rvalues in certain cases (to prefer move +/// construction), then falls back to treating them as lvalues if that failed. /// -/// \param ConvertingConstructorsOnly If true, follow [class.copy]p32 and reject -/// resolutions that find non-constructors, such as derived-to-base conversions -/// or `operator T()&&` member functions. If false, do consider such +/// \param ConvertingConstructorsOnly If true, follow [class.copy.elision]p3 and +/// reject resolutions that find non-constructors, such as derived-to-base +/// conversions or `operator T()&&` member functions. If false, do consider such /// conversion sequences. /// /// \param Res We will fill this in if move-initialization was possible. @@ -3133,10 +3140,14 @@ FunctionDecl *FD = Step.Function.Function; if (ConvertingConstructorsOnly) { - if (isa(FD)) { + if (!isa(FD)) { + continue; + } + if (!S.getLangOpts().CPlusPlus20) { + // C++11 [class.copy]p32: // C++14 [class.copy]p32: - // [...] If the first overload resolution fails or was not performed, - // or if the type of the first parameter of the selected constructor + // C++17 [class.copy.elision]p3: + // [...] if the type of the first parameter of the selected constructor // is not an rvalue reference to the object's type (possibly // cv-qualified), overload resolution is performed again, considering // the object as an lvalue. @@ -3147,8 +3158,6 @@ if (!S.Context.hasSameUnqualifiedType(RRefType->getPointeeType(), NRVOCandidate->getType())) break; - } else { - continue; } } else { if (isa(FD)) { @@ -3182,38 +3191,27 @@ /// Perform the initialization of a potentially-movable value, which /// is the result of return value. /// -/// This routine implements C++14 [class.copy]p32, which attempts to treat -/// returned lvalues as rvalues in certain cases (to prefer move construction), -/// then falls back to treating them as lvalues if that failed. +/// This routine implements C++20 [class.copy.elision]p3, which attempts to +/// treat returned lvalues as rvalues in certain cases (to prefer move +/// construction), then falls back to treating them as lvalues if that failed. ExprResult Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity, const VarDecl *NRVOCandidate, QualType ResultType, Expr *Value, bool AllowNRVO) { - // C++14 [class.copy]p32: - // When the criteria for elision of a copy/move operation are met, but not for - // an exception-declaration, and the object to be copied is designated by an - // lvalue, or when the expression in a return statement is a (possibly - // parenthesized) id-expression that names an object with automatic storage - // duration declared in the body or parameter-declaration-clause of the - // innermost enclosing function or lambda-expression, overload resolution to - // select the constructor for the copy is first performed as if the object - // were designated by an rvalue. ExprResult Res = ExprError(); if (AllowNRVO) { - bool AffectedByCWG1579 = false; + CopyElisionSemanticsKind CESK = CES_Strict; + if (getLangOpts().CPlusPlus20) { + CESK = CES_ImplicitlyMovableCXX20; + } else if (getLangOpts().CPlusPlus11) { + CESK = CES_ImplicitlyMovableCXX11CXX14CXX17; + } if (!NRVOCandidate) { - NRVOCandidate = getCopyElisionCandidate(ResultType, Value, CES_Default); - if (NRVOCandidate && - !getDiagnostics().isIgnored(diag::warn_return_std_move_in_cxx11, - Value->getExprLoc())) { - const VarDecl *NRVOCandidateInCXX11 = - getCopyElisionCandidate(ResultType, Value, CES_FormerDefault); - AffectedByCWG1579 = (!NRVOCandidateInCXX11); - } + NRVOCandidate = getCopyElisionCandidate(ResultType, Value, CESK); } if (NRVOCandidate) { @@ -3221,32 +3219,9 @@ true, Res); } - if (!Res.isInvalid() && AffectedByCWG1579) { - QualType QT = NRVOCandidate->getType(); - if (QT.getNonReferenceType() - .getUnqualifiedType() - .isTriviallyCopyableType(Context)) { - // Adding 'std::move' around a trivially copyable variable is probably - // pointless. Don't suggest it. - } else { - // Common cases for this are returning unique_ptr from a - // function of return type unique_ptr, or returning T from a - // function of return type Expected. This is totally fine in a - // post-CWG1579 world, but was not fine before. - assert(!ResultType.isNull()); - SmallString<32> Str; - Str += "std::move("; - Str += NRVOCandidate->getDeclName().getAsString(); - Str += ")"; - Diag(Value->getExprLoc(), diag::warn_return_std_move_in_cxx11) - << Value->getSourceRange() - << NRVOCandidate->getDeclName() << ResultType << QT; - Diag(Value->getExprLoc(), diag::note_add_std_move_in_cxx11) - << FixItHint::CreateReplacement(Value->getSourceRange(), Str); - } - } else if (Res.isInvalid() && - !getDiagnostics().isIgnored(diag::warn_return_std_move, - Value->getExprLoc())) { + if (!getLangOpts().CPlusPlus20 && Res.isInvalid() && + !getDiagnostics().isIgnored(diag::warn_return_std_move, + Value->getExprLoc())) { const VarDecl *FakeNRVOCandidate = getCopyElisionCandidate(QualType(), Value, CES_AsIfByStdMove); if (FakeNRVOCandidate) { 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 --- /dev/null +++ b/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp @@ -0,0 +1,144 @@ +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -verify %s +// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -verify %s +// RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -verify %s +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -verify %s + +// In C++20, implicitly movable entity can be rvalue reference to non-volatile +// automatic object. +namespace test_implicitly_movable_rvalue_ref { +#if __cplusplus >= 202002L +class C { +public: + C() {} + ~C() {} + C(C &&); + +private: + C(const C &); +}; + +C test(C &&c) { + return c; +} +#else +class C { +public: + C() {} + ~C() {} + C(C &&); + +private: + C(const C &); // expected-note {{declared private here}} +}; + +C test(C &&c) { + return c; // expected-error {{calling a private constructor of class 'test_implicitly_movable_rvalue_ref::C'}} +} +#endif +} // namespace test_implicitly_movable_rvalue_ref + +// In C++20, operand of throw-expression can be function parameter or +// catch-clause parameter. +namespace test_throw_parameter { +#if __cplusplus >= 202002L +class C { +public: + C() {} + ~C() {} + C(const C &); + +private: + C(C &&); // expected-note {{declared private here}} +}; + +void func(); +void test() { + try { + func(); + } catch (C c_move) { + throw c_move; // expected-error {{calling a private constructor of class 'test_throw_parameter::C'}} + } +} +#else +class C { +public: + C() {} + ~C() {} + C(const C &); + +private: + C(C &&); +}; + +void func(); +void test() { + try { + func(); + } catch (C c_copy) { + throw c_copy; + } +} +#endif +} // namespace test_throw_parameter + +// In C++20, during the first overload resolution, the first parameter of the +// selected constructor no need to be rvalue reference to the object's type. +namespace test_ctor_param_rvalue_ref { +class B; + +class NeedRvalueRef { +public: + NeedRvalueRef() {} + ~NeedRvalueRef() {} + NeedRvalueRef(B &&); +}; + +class NeedValue { +public: + NeedValue() {} + ~NeedValue() {} + NeedValue(B); +}; + +#if __cplusplus >= 202002L +class B { +public: + B() {} + ~B() {} + B(B &&); + +private: + B(const B &); +}; + +NeedRvalueRef test1() { + B b_move; + return b_move; +} + +NeedValue test2() { + B b_move; + return b_move; +} +#else +class B { +public: + B() {} + ~B() {} + B(B &&); + +private: + B(const B &); // expected-note {{declared private here}} +}; + +NeedRvalueRef test1() { + B b_move; + return b_move; +} + +NeedValue test2() { + B b_copy; + return b_copy; // expected-error {{calling a private constructor of class 'test_ctor_param_rvalue_ref::B'}} +} +#endif +} // namespace test_ctor_param_rvalue_ref diff --git a/clang/test/SemaCXX/warn-return-std-move.cpp b/clang/test/SemaCXX/warn-return-std-move.cpp --- a/clang/test/SemaCXX/warn-return-std-move.cpp +++ b/clang/test/SemaCXX/warn-return-std-move.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -Wreturn-std-move-in-c++11 -std=c++14 -verify %s -// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -Wreturn-std-move-in-c++11 -std=c++14 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -verify %s +// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -std=c++14 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s // definitions for std::move namespace std { @@ -77,10 +77,7 @@ } ConstructFromDerived test3() { Derived d3; - return d3; // e2-cxx11 - // expected-warning@-1{{would have been copied despite being returned by name}} - // expected-note@-2{{to avoid copying on older compilers}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d3)" + return d3; // ok } ConstructFromBase test4() { Derived d4; @@ -153,10 +150,7 @@ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"std::move(d)" } ConstructFromDerived testParam3(Derived d) { - return d; // e7-cxx11 - // expected-warning@-1{{would have been copied despite being returned by name}} - // expected-note@-2{{to avoid copying on older compilers}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"std::move(d)" + return d; // ok } ConstructFromBase testParam4(Derived d) { return d; // e8 @@ -231,10 +225,7 @@ } ConstructFromDerived testParens2() { Derived d; - return (d); // e18-cxx11 - // expected-warning@-1{{would have been copied despite being returned by name}} - // expected-note@-2{{to avoid copying}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:15}:"std::move(d)" + return (d); // ok } diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -1202,6 +1202,11 @@ P0593R6 (DR) Clang 11 + + More implicit moves + P1825R0 (DR) + Clang 12 +