[clang] 03282f2 - [clang] C++98 implicit moves are back with a vengeance

Matheus Izvekov via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 13 10:16:59 PDT 2021


Author: Matheus Izvekov
Date: 2021-07-13T19:16:49+02:00
New Revision: 03282f2fe14e9dd61aaeeda3785f56c7ccb4f3c9

URL: https://github.com/llvm/llvm-project/commit/03282f2fe14e9dd61aaeeda3785f56c7ccb4f3c9
DIFF: https://github.com/llvm/llvm-project/commit/03282f2fe14e9dd61aaeeda3785f56c7ccb4f3c9.diff

LOG: [clang] C++98 implicit moves are back with a vengeance

After taking C++98 implicit moves out in 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.

* 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.
  * Objects with different type from the function return type.
* 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 operator 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 &); };
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 <mizvekov at gmail.com>

Reviewed By: Quuxplusone

Differential Revision: https://reviews.llvm.org/D105756

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index fa798c2d557ca..643dde437fc94 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3451,6 +3451,28 @@ const VarDecl *Sema::getCopyElisionCandidate(NamedReturnInfo &Info,
   return Info.isCopyElidable() ? Info.Candidate : nullptr;
 }
 
+/// Verify that the initialization sequence that was picked for the
+/// first overload resolution is permissible under C++98.
+///
+/// Reject (possibly converting) contructors not taking an rvalue reference,
+/// or user conversion operators which are not ref-qualified.
+static bool
+VerifyInitializationSequenceCXX98(const Sema &S,
+                                  const InitializationSequence &Seq) {
+  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)
+      return false;
+  }
+  return true;
+}
+
 /// Perform the initialization of a potentially-movable value, which
 /// is the result of return value.
 ///
@@ -3461,8 +3483,7 @@ ExprResult
 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;
@@ -3470,7 +3491,9 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
                                                Value->getBeginLoc());
     InitializationSequence Seq(*this, Entity, Kind, InitExpr);
     auto Res = Seq.getFailedOverloadResult();
-    if (Res == OR_Success || Res == OR_Deleted) {
+    if ((Res == OR_Success || Res == OR_Deleted) &&
+        (getLangOpts().CPlusPlus11 ||
+         VerifyInitializationSequenceCXX98(*this, Seq))) {
       // Promote "AsRvalue" to the heap, since we now need this
       // expression node to persist.
       Value =

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
index a85475ece7bf5..cd981264c9b69 100644
--- a/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
+++ b/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
@@ -7,11 +7,11 @@ namespace test_delete_function {
 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 @@ struct A2 {
   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 @@ B2 test4() {
 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 @@ void func();
 
 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 @@ struct A2 {
   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 @@ class C {};
 
 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 NeedValue {
 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 @@ A1 test_1_2() {
   // 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 @@ struct A2 {
   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 @@ A2 test_2_2() {
   // 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 @@ struct B1 {
   B1();
   B1(const B1 &);
   B1(B1 &&) = delete; // cxx11_2b-note 3 {{'B1' has been explicitly marked deleted here}}
+                      // cxx98-note at -1 {{'B1' has been explicitly marked deleted here}}
 };
 NeedValue test_3_1() {
   // not rvalue reference
@@ -262,7 +263,7 @@ B1 test_3_2() {
   // 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 @@ struct B2 {
 
 private:
   B2(B2 &&); // cxx11_2b-note 3 {{declared private here}}
+             // cxx98-note at -1 {{declared private here}}
 };
 NeedValue test_4_1() {
   // not rvalue reference
@@ -289,7 +291,7 @@ B2 test_4_2() {
   // 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 @@ NeedValue test_4_3() {
 namespace test_lvalue_ref_is_not_moved_from {
 
 struct Target {};
-// cxx11_2b-note at -1  {{candidate constructor (the implicit copy constructor) not viable}}
-// cxx98-note at -2    2{{candidate constructor (the implicit copy constructor) not viable}}
-// cxx11_2b-note at -3  {{candidate constructor (the implicit move constructor) not viable}}
+// expected-note at -1  {{candidate constructor (the implicit copy constructor) not viable}}
+// cxx11_2b-note at -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 t1() {
 
 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 t5() {
 
 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 t7() {
 
 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 @@ Target t4() {
 
 } // namespace test_rvalue_ref_to_nonobject
 
+// Both tests in test_constandnonconstcopy, and also test_conversion::test1, 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 @@ A test1(B x) { return x; } // cxx98-error-re {{conversion {{.*}} is ambiguous}}
 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
 

diff  --git a/clang/test/SemaCXX/conversion-function.cpp b/clang/test/SemaCXX/conversion-function.cpp
index 8ff709ddbbb25..98ee2aba4bc4c 100644
--- a/clang/test/SemaCXX/conversion-function.cpp
+++ b/clang/test/SemaCXX/conversion-function.cpp
@@ -120,7 +120,9 @@ void f(Yb& a) {
   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 {

diff  --git a/clang/test/SemaObjCXX/block-capture.mm b/clang/test/SemaObjCXX/block-capture.mm
index 77a3907c6578c..c89c088a871ba 100644
--- a/clang/test/SemaObjCXX/block-capture.mm
+++ b/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);


        


More information about the cfe-commits mailing list