[PATCH] D105518: [clang] disable P2266 simpler implicit moves under -fms-compatibility

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 357078.
mizvekov added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105518

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp


Index: clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fcxx-exceptions                    -verify=new %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fcxx-exceptions -fms-compatibility -verify=old %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions                    -verify=old %s
+
+// FIXME: This is a test for a temporary workaround where we disable simpler implicit moves
+//        when compiling with -fms-compatibility, because the MSVC STL does not compile.
+//        A better workaround is under discussion.
+//        The test cases here are just a copy from `CXX/class/class.init/class.copy.elision/p3.cpp`,
+//        so feel free to delete this file when the workaround is not needed anymore.
+
+struct CopyOnly {
+  CopyOnly(); // new-note {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
+  // new-note at -1 {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
+  CopyOnly(CopyOnly &); // new-note {{candidate constructor not viable: expects an lvalue for 1st argument}}
+  // new-note at -1 {{candidate constructor not viable: expects an lvalue for 1st argument}}
+};
+struct MoveOnly {
+  MoveOnly();
+  MoveOnly(MoveOnly &&);
+};
+MoveOnly &&rref();
+
+MoveOnly &&test1(MoveOnly &&w) {
+  return w; // old-error {{cannot bind to lvalue of type}}
+}
+
+CopyOnly test2(bool b) {
+  static CopyOnly w1;
+  CopyOnly w2;
+  if (b) {
+    return w1;
+  } else {
+    return w2; // new-error {{no matching constructor for initialization}}
+  }
+}
+
+template <class T> T &&test3(T &&x) { return x; } // old-error {{cannot bind to lvalue of type}}
+template MoveOnly& test3<MoveOnly&>(MoveOnly&);
+template MoveOnly &&test3<MoveOnly>(MoveOnly &&); // old-note {{in instantiation of function template specialization}}
+
+MoveOnly &&test4() {
+  MoveOnly &&x = rref();
+  return x; // old-error {{cannot bind to lvalue of type}}
+}
+
+void test5() try {
+  CopyOnly x;
+  throw x; // new-error {{no matching constructor for initialization}}
+} catch (...) {
+}
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3333,8 +3333,13 @@
   if (!VD)
     return NamedReturnInfo();
   NamedReturnInfo Res = getNamedReturnInfo(VD);
+  // FIXME: We supress simpler implicit move here (unless ForceCXX2b is true)
+  //        in msvc compatibility mode just as a temporary work around,
+  //        as the MSVC STL has issues with this change.
+  //        We will come back later with a more targeted approach.
   if (Res.Candidate && !E->isXValue() &&
-      (ForceCXX2b || getLangOpts().CPlusPlus2b)) {
+      (ForceCXX2b ||
+       (getLangOpts().CPlusPlus2b && !getLangOpts().MSVCCompat))) {
     E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
                                  CK_NoOp, E, nullptr, VK_XValue,
                                  FPOptionsOverride());
Index: clang/lib/Frontend/InitPreprocessor.cpp
===================================================================
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -598,7 +598,8 @@
   }
   // C++2b features.
   if (LangOpts.CPlusPlus2b) {
-    Builder.defineMacro("__cpp_implicit_move", "202011L");
+    if (!LangOpts.MSVCCompat)
+      Builder.defineMacro("__cpp_implicit_move", "202011L");
     Builder.defineMacro("__cpp_size_t_suffix", "202011L");
   }
   if (LangOpts.Char8)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105518.357078.patch
Type: text/x-patch
Size: 3727 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210707/49d8a79a/attachment.bin>