mizvekov created this revision. mizvekov edited the summary of this revision. mizvekov edited the summary of this revision. mizvekov edited the summary of this revision. mizvekov updated this revision to Diff 357688. mizvekov added a comment. mizvekov updated this revision to Diff 357692. mizvekov updated this revision to Diff 357693. mizvekov updated this revision to Diff 357748. mizvekov edited the summary of this revision. mizvekov added a reviewer: rsmith. mizvekov added subscribers: Quuxplusone, jyknight, ldionne. mizvekov published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits.
clang-tidy. mizvekov added a comment. . mizvekov added a comment. . mizvekov added a comment. . After taking C++98 implicit moves out in D104500 <https://reviews.llvm.org/D104500>, we put it back in, but now in a new form which preserves compatibility with pure C++98 programs, while at the same time giving almost all the goodies from P1825 <https://reviews.llvm.org/P1825>. - We use the exact same rules as C++20 with regards to which id-expressions are move eligible. The previous incarnation would only benefit from the proper subset which is copy ellidable. This means we can implicit move, in addition: - Parameters. - RValue references. - Exception variables. - Variables with higher-than-natural required alignment. - We preserve the two-overload resolution, with one small tweak to the first one: If we either pick a (possibly converting) constructor which does not take an rvalue reference, or a user conversion which is not ref-qualified, we abort into the second overload resolution. This gives C++98 almost all the implicit move patterns which we had created test cases for, while at the same time preserving the meaning of these three patterns, which are found in pure C++98 programs: - Classes with both const and non-const copy constructors, but no move constructors, continue to have their non-const copy constructor selected. - We continue to reject as ambiguous the following pattern: struct A { A(B &) = delete; }; struct B { operator A(); }; A foo(B x) { return x; } - We continue to pick the copy constructor in the following pattern: class AutoPtrRef { }; struct AutoPtr { AutoPtr(AutoPtr &); AutoPtr(); AutoPtr(AutoPtrRef); operator AutoPtrRef(); }; AutoPtr test_auto_ptr() { AutoPtr p; return p; } Signed-off-by: Matheus Izvekov <mizve...@gmail.com> Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D105756 Files: clang/lib/Sema/SemaStmt.cpp clang/test/CXX/class/class.init/class.copy.elision/p3.cpp clang/test/SemaCXX/conversion-function.cpp clang/test/SemaObjCXX/block-capture.mm
Index: clang/test/SemaObjCXX/block-capture.mm =================================================================== --- clang/test/SemaObjCXX/block-capture.mm +++ clang/test/SemaObjCXX/block-capture.mm @@ -14,6 +14,10 @@ }; TEST(CopyOnly); // cxx2b-error {{no matching constructor}} +// Both ConstCopyOnly and NonConstCopyOnly are +// "pure" C++98 tests (pretend 'delete' means 'private'). +// However we may extend implicit moves into C++98, we must make sure the +// results in these are not changed. struct ConstCopyOnly { ConstCopyOnly(); ConstCopyOnly(ConstCopyOnly &) = delete; // cxx98-note {{marked deleted here}} @@ -31,51 +35,51 @@ struct CopyNoMove { CopyNoMove(); CopyNoMove(CopyNoMove &); - CopyNoMove(CopyNoMove &&) = delete; // cxx11_2b-note {{marked deleted here}} + CopyNoMove(CopyNoMove &&) = delete; // cxx98_2b-note {{marked deleted here}} }; -TEST(CopyNoMove); // cxx11_2b-error {{call to deleted constructor}} +TEST(CopyNoMove); // cxx98_2b-error {{call to deleted constructor}} struct MoveOnly { MoveOnly(); - MoveOnly(MoveOnly &) = delete; // cxx98-note {{marked deleted here}} + MoveOnly(MoveOnly &) = delete; MoveOnly(MoveOnly &&); }; -TEST(MoveOnly); // cxx98-error {{call to deleted constructor}} +TEST(MoveOnly); struct NoCopyNoMove { NoCopyNoMove(); - NoCopyNoMove(NoCopyNoMove &) = delete; // cxx98-note {{marked deleted here}} - NoCopyNoMove(NoCopyNoMove &&) = delete; // cxx11_2b-note {{marked deleted here}} + NoCopyNoMove(NoCopyNoMove &) = delete; + NoCopyNoMove(NoCopyNoMove &&) = delete; // cxx98_2b-note {{marked deleted here}} }; TEST(NoCopyNoMove); // cxx98_2b-error {{call to deleted constructor}} struct ConvertingRVRef { ConvertingRVRef(); - ConvertingRVRef(ConvertingRVRef &) = delete; // cxx98-note {{marked deleted here}} + ConvertingRVRef(ConvertingRVRef &) = delete; struct X {}; ConvertingRVRef(X &&); operator X() const & = delete; operator X() &&; }; -TEST(ConvertingRVRef); // cxx98-error {{call to deleted constructor}} +TEST(ConvertingRVRef); struct ConvertingCLVRef { ConvertingCLVRef(); ConvertingCLVRef(ConvertingCLVRef &); struct X {}; - ConvertingCLVRef(X &&); // cxx11_2b-note {{passing argument to parameter here}} + ConvertingCLVRef(X &&); // cxx98_2b-note {{passing argument to parameter here}} operator X() const &; - operator X() && = delete; // cxx11_2b-note {{marked deleted here}} + operator X() && = delete; // cxx98_2b-note {{marked deleted here}} }; -TEST(ConvertingCLVRef); // cxx11_2b-error {{invokes a deleted function}} +TEST(ConvertingCLVRef); // cxx98_2b-error {{invokes a deleted function}} struct SubSubMove {}; struct SubMove : SubSubMove { SubMove(); - SubMove(SubMove &) = delete; // cxx98-note {{marked deleted here}} + SubMove(SubMove &) = delete; SubMove(SubSubMove &&); }; -TEST(SubMove); // cxx98-error {{call to deleted constructor}} +TEST(SubMove); Index: clang/test/SemaCXX/conversion-function.cpp =================================================================== --- clang/test/SemaCXX/conversion-function.cpp +++ clang/test/SemaCXX/conversion-function.cpp @@ -120,7 +120,9 @@ char ch = a; // OK. calls Yb::operator char(); } -// Test conversion + copy construction. +// Test conversion + copy construction. This is a pure C++98 test. +// However we may extend implicit moves into C++98, we must make sure the +// result here is not changed. class AutoPtrRef { }; class AutoPtr { Index: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp =================================================================== --- clang/test/CXX/class/class.init/class.copy.elision/p3.cpp +++ clang/test/CXX/class/class.init/class.copy.elision/p3.cpp @@ -7,11 +7,11 @@ struct A1 { A1(); A1(const A1 &); - A1(A1 &&) = delete; // cxx11_2b-note {{'A1' has been explicitly marked deleted here}} + A1(A1 &&) = delete; // expected-note {{'A1' has been explicitly marked deleted here}} }; A1 test1() { A1 a; - return a; // cxx11_2b-error {{call to deleted constructor of 'test_delete_function::A1'}} + return a; // expected-error {{call to deleted constructor of 'test_delete_function::A1'}} } struct A2 { @@ -19,33 +19,33 @@ A2(const A2 &); private: - A2(A2 &&); // cxx11_2b-note {{declared private here}} + A2(A2 &&); // expected-note {{declared private here}} }; A2 test2() { A2 a; - return a; // cxx11_2b-error {{calling a private constructor of class 'test_delete_function::A2'}} + return a; // expected-error {{calling a private constructor of class 'test_delete_function::A2'}} } struct C {}; struct B1 { B1(C &); - B1(C &&) = delete; // cxx11_2b-note {{'B1' has been explicitly marked deleted here}} + B1(C &&) = delete; // expected-note {{'B1' has been explicitly marked deleted here}} }; B1 test3() { C c; - return c; // cxx11_2b-error {{conversion function from 'test_delete_function::C' to 'test_delete_function::B1' invokes a deleted function}} + 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 &&); // cxx11_2b-note {{declared private here}} + B2(C &&); // expected-note {{declared private here}} }; B2 test4() { C c; - return c; // cxx11_2b-error {{calling a private constructor of class 'test_delete_function::B2'}} + return c; // expected-error {{calling a private constructor of class 'test_delete_function::B2'}} } } // namespace test_delete_function @@ -54,38 +54,38 @@ namespace test_implicitly_movable_rvalue_ref { struct A1 { A1(A1 &&); - A1(const A1 &) = delete; // cxx98-note {{marked deleted here}} + A1(const A1 &) = delete; }; A1 test1(A1 &&a) { - return a; // cxx98-error {{call to deleted constructor}} + return a; } struct A2 { A2(A2 &&); private: - A2(const A2 &); // cxx98-note {{declared private here}} + A2(const A2 &); }; A2 test2(A2 &&a) { - return a; // cxx98-error {{calling a private constructor}} + return a; } struct B1 { B1(const B1 &); - B1(B1 &&) = delete; // cxx11_2b-note {{'B1' has been explicitly marked deleted here}} + B1(B1 &&) = delete; // expected-note {{'B1' has been explicitly marked deleted here}} }; B1 test3(B1 &&b) { - return b; // cxx11_2b-error {{call to deleted constructor of 'test_implicitly_movable_rvalue_ref::B1'}} + return b; // expected-error {{call to deleted constructor of 'test_implicitly_movable_rvalue_ref::B1'}} } struct B2 { B2(const B2 &); private: - B2(B2 &&); // cxx11_2b-note {{declared private here}} + B2(B2 &&); // expected-note {{declared private here}} }; B2 test4(B2 &&b) { - return b; // cxx11_2b-error {{calling a private constructor of class 'test_implicitly_movable_rvalue_ref::B2'}} + return b; // expected-error {{calling a private constructor of class 'test_implicitly_movable_rvalue_ref::B2'}} } } // namespace test_implicitly_movable_rvalue_ref @@ -96,13 +96,13 @@ struct A1 { A1(const A1 &); - A1(A1 &&) = delete; // cxx11_2b-note 2{{'A1' has been explicitly marked deleted here}} + A1(A1 &&) = delete; // expected-note 2{{'A1' has been explicitly marked deleted here}} }; void test1() { try { func(); } catch (A1 a) { - throw a; // cxx11_2b-error {{call to deleted constructor of 'test_throw_parameter::A1'}} + throw a; // expected-error {{call to deleted constructor of 'test_throw_parameter::A1'}} } } @@ -110,20 +110,20 @@ A2(const A2 &); private: - A2(A2 &&); // cxx11_2b-note {{declared private here}} + A2(A2 &&); // expected-note {{declared private here}} }; void test2() { try { func(); } catch (A2 a) { - throw a; // cxx11_2b-error {{calling a private constructor of class 'test_throw_parameter::A2'}} + throw a; // expected-error {{calling a private constructor of class 'test_throw_parameter::A2'}} } } void test3(A1 a) try { func(); } catch (...) { - throw a; // cxx11_2b-error {{call to deleted constructor of 'test_throw_parameter::A1'}} + throw a; // expected-error {{call to deleted constructor of 'test_throw_parameter::A1'}} } } // namespace test_throw_parameter @@ -134,42 +134,42 @@ struct A1 { operator C() &&; - operator C() const & = delete; // cxx98-note {{marked deleted here}} + operator C() const & = delete; }; C test1() { A1 a; - return a; // cxx98-error {{invokes a deleted function}} + return a; } struct A2 { operator C() &&; private: - operator C() const &; // cxx98-note {{declared private here}} + operator C() const &; }; C test2() { A2 a; - return a; // cxx98-error {{'operator C' is a private member}} + return a; } struct B1 { operator C() const &; - operator C() && = delete; // cxx11_2b-note {{'operator C' has been explicitly marked deleted here}} + operator C() && = delete; // expected-note {{'operator C' has been explicitly marked deleted here}} }; C test3() { B1 b; - return b; // cxx11_2b-error {{conversion function from 'test_non_ctor_conversion::B1' to 'test_non_ctor_conversion::C' invokes a deleted function}} + return b; // expected-error {{conversion function from 'test_non_ctor_conversion::B1' to 'test_non_ctor_conversion::C' invokes a deleted function}} } struct B2 { operator C() const &; private: - operator C() &&; // cxx11_2b-note {{declared private here}} + operator C() &&; // expected-note {{declared private here}} }; C test4() { B2 b; - return b; // cxx11_2b-error {{'operator C' is a private member of 'test_non_ctor_conversion::B2'}} + return b; // expected-error {{'operator C' is a private member of 'test_non_ctor_conversion::B2'}} } } // namespace test_non_ctor_conversion @@ -197,7 +197,7 @@ struct A1 { A1(); A1(A1 &&); - A1(const A1 &) = delete; // cxx98-note 3{{marked deleted here}} + A1(const A1 &) = delete; // cxx98-note 2 {{marked deleted here}} }; NeedValue test_1_1() { // not rvalue reference @@ -210,7 +210,7 @@ // rvalue reference // not same type DerivedA1 a; - return a; // cxx98-error {{call to deleted constructor}} + return a; } NeedValue test_1_3() { // not rvalue reference @@ -224,7 +224,7 @@ A2(A2 &&); private: - A2(const A2 &); // cxx98-note 3{{declared private here}} + A2(const A2 &); // cxx98-note 2 {{declared private here}} }; NeedValue test_2_1() { // not rvalue reference @@ -237,7 +237,7 @@ // rvalue reference // not same type DerivedA2 a; - return a; // cxx98-error {{calling a private constructor}} + return a; } NeedValue test_2_3() { // not rvalue reference @@ -250,6 +250,7 @@ B1(); B1(const B1 &); B1(B1 &&) = delete; // cxx11_2b-note 3 {{'B1' has been explicitly marked deleted here}} + // cxx98-note@-1 {{'B1' has been explicitly marked deleted here}} }; NeedValue test_3_1() { // not rvalue reference @@ -262,7 +263,7 @@ // rvalue reference // not same type DerivedB1 b; - return b; // cxx11_2b-error {{call to deleted constructor of 'test_ctor_param_rvalue_ref::B1'}} + return b; // expected-error {{call to deleted constructor of 'test_ctor_param_rvalue_ref::B1'}} } NeedValue test_3_3() { // not rvalue reference @@ -277,6 +278,7 @@ private: B2(B2 &&); // cxx11_2b-note 3 {{declared private here}} + // cxx98-note@-1 {{declared private here}} }; NeedValue test_4_1() { // not rvalue reference @@ -289,7 +291,7 @@ // rvalue reference // not same type DerivedB2 b; - return b; // cxx11_2b-error {{calling a private constructor of class 'test_ctor_param_rvalue_ref::B2'}} + return b; // expected-error {{calling a private constructor of class 'test_ctor_param_rvalue_ref::B2'}} } NeedValue test_4_3() { // not rvalue reference @@ -302,20 +304,19 @@ namespace test_lvalue_ref_is_not_moved_from { struct Target {}; -// cxx11_2b-note@-1 {{candidate constructor (the implicit copy constructor) not viable}} -// cxx98-note@-2 2{{candidate constructor (the implicit copy constructor) not viable}} -// cxx11_2b-note@-3 {{candidate constructor (the implicit move constructor) not viable}} +// expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable}} +// cxx11_2b-note@-2 {{candidate constructor (the implicit move constructor) not viable}} struct CopyOnly { - CopyOnly(CopyOnly &&) = delete; // cxx11_2b-note {{has been explicitly marked deleted here}} + CopyOnly(CopyOnly &&) = delete; // expected-note {{has been explicitly marked deleted here}} CopyOnly(CopyOnly&); - operator Target() && = delete; // cxx11_2b-note {{has been explicitly marked deleted here}} + operator Target() && = delete; // expected-note {{has been explicitly marked deleted here}} operator Target() &; }; struct MoveOnly { MoveOnly(MoveOnly &&); // cxx11_2b-note {{copy constructor is implicitly deleted because}} - operator Target() &&; // expected-note {{candidate function not viable}} cxx98-note {{candidate function not viable}} + operator Target() &&; // expected-note {{candidate function not viable}} }; extern CopyOnly copyonly; @@ -328,7 +329,7 @@ CopyOnly t2() { CopyOnly&& r = static_cast<CopyOnly&&>(copyonly); - return r; // cxx11_2b-error {{call to deleted constructor}} + return r; // expected-error {{call to deleted constructor}} } MoveOnly t3() { @@ -348,7 +349,7 @@ Target t6() { CopyOnly&& r = static_cast<CopyOnly&&>(copyonly); - return r; // cxx11_2b-error {{invokes a deleted function}} + return r; // expected-error {{invokes a deleted function}} } Target t7() { @@ -358,7 +359,7 @@ Target t8() { MoveOnly&& r = static_cast<MoveOnly&&>(moveonly); - return r; // cxx98-error {{no viable conversion}} + return r; } } // namespace test_lvalue_ref_is_not_moved_from @@ -400,6 +401,10 @@ } // namespace test_rvalue_ref_to_nonobject +// Both tests in test_constandnonconstcopy, and also test_conversion::t1, are +// "pure" C++98 tests (pretend 'delete' means 'private'). +// However we may extend implicit moves into C++98, we must make sure the +// results in these are not changed. namespace test_constandnonconstcopy { struct ConstCopyOnly { ConstCopyOnly(); @@ -437,9 +442,9 @@ struct C {}; struct D { operator C() &; - operator C() const & = delete; // cxx11_2b-note {{marked deleted here}} + operator C() const & = delete; // expected-note {{marked deleted here}} }; -C test2(D x) { return x; } // cxx11_2b-error {{invokes a deleted function}} +C test2(D x) { return x; } // expected-error {{invokes a deleted function}} } // namespace test_conversion Index: clang/lib/Sema/SemaStmt.cpp =================================================================== --- clang/lib/Sema/SemaStmt.cpp +++ clang/lib/Sema/SemaStmt.cpp @@ -3461,8 +3461,7 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity, const NamedReturnInfo &NRInfo, Expr *Value) { - if (getLangOpts().CPlusPlus11 && !getLangOpts().CPlusPlus2b && - NRInfo.isMoveEligible()) { + if (!getLangOpts().CPlusPlus2b && NRInfo.isMoveEligible()) { ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(), CK_NoOp, Value, VK_XValue, FPOptionsOverride()); Expr *InitExpr = &AsRvalue; @@ -3471,6 +3470,20 @@ InitializationSequence Seq(*this, Entity, Kind, InitExpr); auto Res = Seq.getFailedOverloadResult(); if (Res == OR_Success || Res == OR_Deleted) { + if (!getLangOpts().CPlusPlus11) { + const auto *Step = llvm::find_if(Seq.steps(), [](const auto &Step) { + return Step.Kind == + InitializationSequence::SK_ConstructorInitialization || + Step.Kind == InitializationSequence::SK_UserConversion; + }); + if (Step != Seq.step_end()) { + const auto *FD = Step->Function.Function; + if (isa<CXXConstructorDecl>(FD) + ? !FD->getParamDecl(0)->getType()->isRValueReferenceType() + : cast<CXXMethodDecl>(FD)->getRefQualifier() == RQ_None) + goto second_overload_resolution; + } + } // Promote "AsRvalue" to the heap, since we now need this // expression node to persist. Value = @@ -3481,6 +3494,7 @@ return Seq.Perform(*this, Entity, Kind, Value); } } +second_overload_resolution: // 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.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits