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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone accepted this revision.
Quuxplusone added a comment.
This revision is now accepted and ready to land.

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



================
Comment at: clang/lib/Sema/SemaStmt.cpp:3484
+                  : cast<CXXMethodDecl>(FD)->getRefQualifier() == RQ_None)
+            goto second_overload_resolution;
+        }
----------------
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



================
Comment at: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp:404
 
+// Both tests in test_constandnonconstcopy, and also test_conversion::t1, are
+// "pure" C++98 tests (pretend 'delete' means 'private').
----------------
/t1/test1/?
Thanks for this note; this answers my question about whether we're testing the problematic cases. :)


================
Comment at: clang/test/SemaObjCXX/block-capture.mm:40
 };
-TEST(CopyNoMove); // cxx11_2b-error {{call to deleted constructor}}
+TEST(CopyNoMove); // cxx98_2b-error {{call to deleted constructor}}
 
----------------
`cxx98_2b` is just `expected`, isn't it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105756/new/

https://reviews.llvm.org/D105756