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

[mizvekov]

Summary

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 <[e-mail]>

[mizvekov]

clang-tidy.

[mizvekov]

.

[mizvekov]

.

[mizvekov]

.

[• Quuxplusone]

It would be an excellent idea to try the libc++ test suite with this patch (make sure to pass --param std=c++03).

clang/lib/Sema/SemaStmt.cpp

3506

It sure would be nice to factor something out into a function here so that we didn't need a goto. Either replace the goto with

return PerformCopyInitialization(Entity, SourceLocation(), Value);

or else pull the Step stuff out into a static bool PermitImplicitMoveSequence(...) or whatever an appropriate name might be:

if ((Res == OR_Success || Res == OR_Deleted) && PermitImplicitMoveSequence(Seq)) {
  // Promote "AsRvalue" to the heap, etc etc

clang/test/CXX/class/class.init/class.copy.elision/p3.cpp

404

/t1/test1/?
Thanks for this note; this answers my question about whether we're testing the problematic cases. :)

clang/test/SemaObjCXX/block-capture.mm

40

cxx98_2b is just expected, isn't it?

[mizvekov]

• Fix minor nits.
• Please Dijkstra.

[mizvekov]

clang/test/SemaObjCXX/block-capture.mm

40

Yeah, but in this test we don't have the "expected" expectancy string.

[NoQ]

It looks like this commit causes unexpected changes in non-C++ code. Eg., in this tiny reproducer (that uses the non-standard "blocks" feature but is otherwise plain C):

typedef struct {
  int x;
} S;

void foo() {
  ^{
    S s;
    return s;
  };
}

I'm suddenly seeing C++-specific items in the AST that shouldn't show up in C (xvalues, NRVO, etc.):

$ clang -x c -Xclang -ast-dump test.c

...
`-FunctionDecl 0x7fc55288aa88 <line:5:1, line:10:1> line:5:6 foo 'void ()'
  `-CompoundStmt 0x7fc55288aec0 <col:12, line:10:1>
    `-BlockExpr 0x7fc55288aea8 <line:6:3, line:9:3> 'S (^)(void)'
      `-BlockDecl 0x7fc55288ab70 <line:6:3, line:9:3> line:6:3
        |-CompoundStmt 0x7fc55288adc8 <col:4, line:9:3>
        | |-DeclStmt 0x7fc55288acd8 <line:7:5, col:8>
        | | `-VarDecl 0x7fc55288ac70 <col:5, col:7> col:7 used s 'S':'S' nrvo
        | `-ReturnStmt 0x7fc55288adb0 <line:8:5, col:12>
        |   `-ImplicitCastExpr 0x7fc55288ad98 <col:12> 'S':'S' <LValueToRValue>
        |     `-ImplicitCastExpr 0x7fc55288ad80 <col:12> 'S':'S' xvalue <NoOp>
        |       `-ImplicitCastExpr 0x7fc55288ad68 <col:12> 'S':'S' <LValueToRValue>
        |         `-DeclRefExpr 0x7fc55288acf0 <col:12> 'S':'S' lvalue Var 0x7fc55288ac70 's' 'S':'S'
        `-VarDecl 0x7fc55288ac70 <line:7:5, col:7> col:7 used s 'S':'S' nrvo

The double implicit lvalue-to-rvalue cast looks incorrect as well (there isn't an extra dereference here).

Here's how the old AST looks like:

...
`-FunctionDecl 0x7fb594079e88 <line:5:1, line:10:1> line:5:6 foo 'void ()'
  `-CompoundStmt 0x7fb59407a230 <col:12, line:10:1>
    `-BlockExpr 0x7fb59407a218 <line:6:3, line:9:3> 'S (^)(void)'
      `-BlockDecl 0x7fb594079f70 <line:6:3, line:9:3> line:6:3
        |-CompoundStmt 0x7fb59407a138 <col:4, line:9:3>
        | |-DeclStmt 0x7fb59407a0d8 <line:7:5, col:8>
        | | `-VarDecl 0x7fb59407a070 <col:5, col:7> col:7 used s 'S':'S'
        | `-ReturnStmt 0x7fb59407a128 <line:8:5, col:12>
        |   `-ImplicitCastExpr 0x7fb59407a110 <col:12> 'S':'S' <LValueToRValue>
        |     `-DeclRefExpr 0x7fb59407a0f0 <col:12> 'S':'S' lvalue Var 0x7fb59407a070 's' 'S':'S'
        `-VarDecl 0x7fb59407a070 <line:7:5, col:7> col:7 used s 'S':'S'

I don't think this is intended. @mizvekov, do you think you can add more hard checks to make sure it's constrained to C++? (Or maybe there's another, more precise solution?)

[mizvekov]

In D105756#2988022, @NoQ wrote:

I don't think this is intended. @mizvekov, do you think you can add more hard checks to make sure it's constrained to C++? (Or maybe there's another, more precise solution?)

Hello! Thanks for reporting this.

The double implicit lvalue-to-rvalue cast is an unfortunate consequence of some technical debt around the different ways we handle the implicit return type on blocks.
But I don't think it's incorrect as in it actually producing program with different semantics.
Does it actually affect you in your use cases besides the AST? Is this important to you for external semantic analysis reasons or something?

Removing the xvalue cast on non CplusPlus is simple, the bigger issue here is why we are doing this, specially as it will affect what test cases we create for this change.
If this is an issue, is it serious enough to consider getting the fix into 13.0.0 at this stage?

The nrvo is a separate thing, and again I thin it's harmless, or at least a little beneficial. With it, you are avoiding an extra copy when returning these values, and it comes straight from the frontend, independent of any optimization passes later.
Why is this undesirable?

[mizvekov]

@NoQ I have made a DR for that change: D109654

[NoQ]

In D105756#2990495, @mizvekov wrote:

Hello! Thanks for reporting this.

Thanks for fixing!

In D105756#2990495, @mizvekov wrote:

The double implicit lvalue-to-rvalue cast is an unfortunate consequence of some technical debt around the different ways we handle the implicit return type on blocks.
But I don't think it's incorrect as in it actually producing program with different semantics.
Does it actually affect you in your use cases besides the AST? Is this important to you for external semantic analysis reasons or something?

I guess I'm confused about the exact meaning of the AST. I'm not sure whether it's actively incorrect or just changed meaning. I'll definitely be debugging this further.

My backstory here is that the static analyzer can be thought of as (primitivized down to) an AST "interpreter". It arranges AST nodes in the order in which they're "executed" at run-time (aka "Clang CFG") and then processes them one node at a time in that order - unlike CodeGen that processes the nodes more or less recursively. This causes us static analyzer people to have an exotic taste for the good AST as we're required to understand every node literally in isolation from other nodes. Every time the nodes make sense together/recursively but not individually our life becomes much harder.

So in the new AST (which wouldn't exist with your fix so we may be talking about vacuous truths here, as I'm still to look at the C++ variant of the same AST) I'm confused about the seemingly brand-new AST node that you introduced, ImplicitCastExpr xvalue <NoOp> - a cast that changes value kind from prvalue to glvalue and in this regard appears to behave exactly like MaterializeTemporaryExpr - which was hard enough to support the first time (due to its deep recursive nature where it extracts the storage address that was already "lost" in the operand), I really don't want to do it again :) And I'm also confused by the situation where ImplicitCastExpr <LValueToRValue> does not correspond to a memory load like it typically does.

If I understand correctly, in C++ the innermost lvalue-to-rvalue cast wouldn't exist, therefore the double memory load concern will be eliminated. I.e., it'll probably be something like

ReturnStmt
`- ImplicitCastExpr <LValueToRValue>
   `- ImplicitCastExpr <NoOp> xvalue  // MaterializeTemporaryExpr xvalue?
      `- CXXConstructExpr

which makes perfect sense to me (assuming MaterializeTemporaryExpr is actually used). I'm assuming the old AST would be

ReturnStmt
`- CXXConstructExpr

which also makes perfect sense. So if my understanding is correct then we're probably mostly good.

In D105756#2990495, @mizvekov wrote:

The nrvo is a separate thing, and again I thin it's harmless, or at least a little beneficial.

I definitely don't mind it.

In D105756#2990495, @mizvekov wrote:

Removing the xvalue cast on non CplusPlus is simple, the bigger issue here is why we are doing this, specially as it will affect what test cases we create for this change.
If this is an issue, is it serious enough to consider getting the fix into 13.0.0 at this stage?

I think your current fix is sufficient. It eliminates the known crash and we're defended against xvalues in C. I'll see what I can do to investigate this further.

[mizvekov]

In D105756#3001045, @NoQ wrote:

I guess I'm confused about the exact meaning of the AST. I'm not sure whether it's actively incorrect or just changed meaning. I'll definitely be debugging this further.

My backstory here is that the static analyzer can be thought of as (primitivized down to) an AST "interpreter". It arranges AST nodes in the order in which they're "executed" at run-time (aka "Clang CFG") and then processes them one node at a time in that order - unlike CodeGen that processes the nodes more or less recursively. This causes us static analyzer people to have an exotic taste for the good AST as we're required to understand every node literally in isolation from other nodes. Every time the nodes make sense together/recursively but not individually our life becomes much harder.

So in the new AST (which wouldn't exist with your fix so we may be talking about vacuous truths here, as I'm still to look at the C++ variant of the same AST) I'm confused about the seemingly brand-new AST node that you introduced, ImplicitCastExpr xvalue <NoOp> - a cast that changes value kind from prvalue to glvalue and in this regard appears to behave exactly like MaterializeTemporaryExpr - which was hard enough to support the first time (due to its deep recursive nature where it extracts the storage address that was already "lost" in the operand), I really don't want to do it again :) And I'm also confused by the situation where ImplicitCastExpr <LValueToRValue> does not correspond to a memory load like it typically does.

The xvalue cast used in NRVO should never apply to a prvalue, as they are used only in expressions that refer to a variable / parameter, which are always lvalues, so it's really only changing categories while still staying within glvalues.

The NoOp cast, as far as code generation / execution is concerned, does nothing really. The purpose of a NoOp cast which changes value category lies in semantic analysis, where it will help things like deciding what kind overload will be selected or what kinds of references an expression can be bound to. Seeing those casts in the middle of a complete AST can only help make sense of why certain overloads where selected, but it would not aid in interpreting how the program should be executed.

If I understand correctly, in C++ the innermost lvalue-to-rvalue cast wouldn't exist, therefore the double memory load concern will be eliminated. I.e., it'll probably be something like

As far as the clang CodeGen is concerned, there is no double memory load here, when starting from
the left in the "LValueToRValue -> NoOp -> LValueToRValue" sequence, the code that implements this in CGExpr EmitLValue function will just recursively eat the NoOp cast (EmitCastLValue) and then go back where it started recursively calling EmitLValue again, making this equivalent to just a single LValueToRValue, as far as what code is actually emitted.

ReturnStmt
`- ImplicitCastExpr <LValueToRValue>
   `- ImplicitCastExpr <NoOp> xvalue  // MaterializeTemporaryExpr xvalue?
      `- CXXConstructExpr

I think that is not correct, in C++ when dealing with classes, the CXXConstructExpr would stand for the LValueToRValue cast there.
The implicit cast to xvalue would also stand between the DeclRef and the CXXConstruct / LValueToRValue.

clang/lib/Sema/SemaStmt.cpp

if (!ReturnType->isDependentType()) {
     // When considering moving this expression out, allow dissimilar types.
     if (!VDType->isDependentType() &&
         !Context.hasSameUnqualifiedType(ReturnType, VDType))
       Info.S = NamedReturnInfo::MoveEligible;
   }
   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.
 ///
 /// 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 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;
     auto Kind = InitializationKind::CreateCopy(Value->getBeginLoc(),
                                                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 =
           ImplicitCastExpr::Create(Context, Value->getType(), CK_NoOp, Value,
                                    nullptr, VK_XValue, FPOptionsOverride());
       // Complete type-checking the initialization of the return type
       // using the constructor we found.
       return Seq.Perform(*this, Entity, Kind, Value);
     }
   }
   // 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.
   return PerformCopyInitialization(Entity, SourceLocation(), Value);
 }
 
 /// Determine whether the declared return type of the specified function
 /// contains 'auto'.

clang/test/CXX/class/class.init/class.copy.elision/p3.cpp

 // RUN: %clang_cc1 -std=c++2b -fsyntax-only -fcxx-exceptions                       -verify=expected,cxx11_2b,cxx2b    %s
 // RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions                       -verify=expected,cxx98_20,cxx11_2b,cxx11_20 %s
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions                       -verify=expected,cxx98_20,cxx11_2b,cxx11_20 %s
 // RUN: %clang_cc1 -std=c++98 -fsyntax-only -fcxx-exceptions -Wno-c++11-extensions -verify=expected,cxx98_20,cxx98 %s
 
 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 {
   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
 
 // Implicitly movable entity can be rvalue reference to non-volatile
 // automatic object.
 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
 
 // Operand of throw-expression can be function parameter or
 // catch-clause parameter.
 namespace test_throw_parameter {
 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'}}
   }
 }
 
 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
 
 // During the first overload resolution, the selected function no
 // need to be a constructor.
 namespace test_non_ctor_conversion {
 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
 
 // During the first overload resolution, the first parameter of the
 // selected function no need to be an rvalue reference to the object's type.
 namespace test_ctor_param_rvalue_ref {
 struct A1;
 struct A2;
struct NeedValue {
   NeedValue(A2);
   NeedValue(B1); // cxx11_2b-note 2 {{passing argument to parameter here}}
   NeedValue(B2);
 };
 
 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
   // same type
   A1 a;
   return a; // cxx98-error {{call to deleted constructor}}
 }
 class DerivedA1 : public A1 {};
 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
   // not same type
   DerivedA1 a;
   return a; // cxx98-error {{call to deleted constructor}}
 }
 
 struct A2 {
   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
   // same type
   A2 a;
   return a; // cxx98-error {{calling a private constructor}}
 }
 class DerivedA2 : public A2 {};
 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
   // not same type
   DerivedA2 a;
   return a; // cxx98-error {{calling a private constructor}}
 }
 
 struct B1 {
   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
   // same type
   B1 b;
   return b; // cxx11_2b-error {{call to deleted constructor of 'test_ctor_param_rvalue_ref::B1'}}
 }
 class DerivedB1 : public B1 {};
 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
   // not same type
   DerivedB1 b;
   return b; // cxx11_2b-error {{call to deleted constructor of 'test_ctor_param_rvalue_ref::B1'}}
 }
 
 struct B2 {
   B2();
   B2(const B2 &);
 
 private:
   B2(B2 &&); // cxx11_2b-note 3 {{declared private here}}
+             // cxx98-note@-1 {{declared private here}}
 };
 NeedValue test_4_1() {
   // not rvalue reference
   // same type
   B2 b;
   return b; // cxx11_2b-error {{calling a private constructor of class 'test_ctor_param_rvalue_ref::B2'}}
 }
 class DerivedB2 : public B2 {};
 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
   // not same type
   DerivedB2 b;
   return b; // cxx11_2b-error {{calling a private constructor of class 'test_ctor_param_rvalue_ref::B2'}}
 }
 } // namespace test_ctor_param_rvalue_ref
 
 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;
 extern MoveOnly moveonly;
 
 CopyOnly t1() {
     CopyOnly& r = copyonly;
     return r;
 }
 
 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() {
     MoveOnly& r = moveonly;
     return r; // cxx11_2b-error {{call to implicitly-deleted copy constructor}}
 }
 
 MoveOnly t4() {
     MoveOnly&& r = static_cast<MoveOnly&&>(moveonly);
     return r;
 }
 
 Target t5() {
     CopyOnly& r = copyonly;
     return r;
 }
 
 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() {
     MoveOnly& r = moveonly;
     return r; // expected-error {{no viable conversion}}
 }
 
 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
 
 namespace test_rvalue_ref_to_nonobject {
 
 struct CopyOnly {};
 struct MoveOnly {};

 
 Target t4() {
     MoveOnly (&&r)() = static_cast<MoveOnly(&&)()>(make_moveonly);
     return r; // expected-error {{invokes a deleted function}}
 }
 
 } // 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();
   ConstCopyOnly(ConstCopyOnly &) = delete; // cxx98-note {{marked deleted here}}
   ConstCopyOnly(const ConstCopyOnly &);
 };
 ConstCopyOnly t1() {
   ConstCopyOnly x;

 struct B {
   operator A(); // cxx98-note {{candidate function}}
 };
 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
 
 namespace test_simpler_implicit_move {
 
 struct CopyOnly {
   CopyOnly(); // cxx2b-note {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
   // cxx2b-note@-1 {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}

clang/test/SemaCXX/conversion-function.cpp

 };
 
 void f(Yb& a) {
   if (a) { } // expected-error {{conversion from 'Yb' to 'bool' is ambiguous}}
   int i = a; // OK. calls XB::operator int();
   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 {
   AutoPtr(AutoPtr &); // cxx98-note {{declared private here}}
 
 public:
   AutoPtr();
   AutoPtr(AutoPtrRef);

clang/test/SemaObjCXX/block-capture.mm

 // RUN: %clang_cc1 -std=c++2b -fsyntax-only -fobjc-arc -fblocks                       -verify=cxx98_2b,cxx11_2b,cxx2b %s
 // RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks                       -verify=cxx98_2b,cxx11_2b       %s
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks                       -verify=cxx98_2b,cxx11_2b       %s
 // RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_2b,cxx98          %s
 
 #define TEST(T) void test_##T() { \
   __block T x;                    \
   (void)^(void) { (void)x; };     \
 }
 
 struct CopyOnly {
   CopyOnly();           // cxx2b-note {{not viable}}
   CopyOnly(CopyOnly &); // cxx2b-note {{not viable}}
 };
 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}}
   ConstCopyOnly(const ConstCopyOnly &);
 };
 TEST(ConstCopyOnly); // cxx98-error {{call to deleted constructor}}
 
 struct NonConstCopyOnly {
   NonConstCopyOnly();
   NonConstCopyOnly(NonConstCopyOnly &);
   NonConstCopyOnly(const NonConstCopyOnly &) = delete; // cxx11_2b-note {{marked deleted here}}
 };
 TEST(NonConstCopyOnly); // cxx11_2b-error {{call to deleted constructor}}
 
 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);