[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Matheus Izvekov via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG20555a15a596: [clang] P2266 implicit moves STL workaround 
(authored by mizvekov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaCoroutine.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
===
--- clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
+++ clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
@@ -1,52 +1,153 @@
-// 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
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only-verify=cxx2b,new %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fms-compatibility -verify=cxx2b,old %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only-verify=cxx20,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@-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@-1 {{candidate constructor not viable: expects an lvalue for 1st argument}}
-};
-struct MoveOnly {
-  MoveOnly();
-  MoveOnly(MoveOnly &&);
+//in the STL when compiling with -fms-compatibility, because of issues with the
+//implementation there.
+//Feel free to delete this file when the workaround is not needed anymore.
+
+#if __INCLUDE_LEVEL__ == 0
+
+#if __cpluscplus > 202002L && __cpp_implicit_move < 202011L
+#error "__cpp_implicit_move not defined correctly"
+#endif
+
+struct nocopy {
+  nocopy(nocopy &&);
 };
-MoveOnly &();
-
-MoveOnly &(MoveOnly &) {
-  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  T &(T &) { return x; } // old-error {{cannot bind to lvalue of type}}
-template MoveOnly (MoveOnly &);
-template MoveOnly &(MoveOnly &&); // old-note {{in instantiation of function template specialization}}
-
-MoveOnly &() {
-  MoveOnly & = 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 (...) {
-}
-
-MoveOnly test6(MoveOnly x) { return x; }
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+
+namespace {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace
+
+namespace foo {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+namespace std {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace std
+} // namespace foo
+
+namespace std {
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+
+namespace {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace
+
+namespace foo {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace foo
+
+} // namespace std
+
+#include __FILE__
+
+#define 

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Melanie Blower via Phabricator via cfe-commits]

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

I tested this patch downstream on Windows with Microsoft (R) C/C++ Optimizing 
Compiler Version 19.28.29334 for x64

I used c++20, x++latest default and c++17 with the stl header files (standalone 
include files--not usage) and didn't find any problems.

c:/Program files (x86)/Microsoft Visual 
Studio/2019/Professional/VC/Tools/MSVC/14.28.29333/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

If you think landing this in 13 after the release branch is created will be 
complicated, then please consider this message a gentle **Ping** :)

Frankly, I am satisfied with the mechanics of this workaround as is.

- After another look, I think doing something more targeted will be unduly 
convoluted, as we would want to target particular function overloads, one which 
is an operator>>.
- The STL will still be tested upstream, with "-fno-mscompatibility".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Matheus Izvekov via Phabricator via cfe-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaCoroutine.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
===
--- clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
+++ clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
@@ -1,52 +1,153 @@
-// 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
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only-verify=cxx2b,new %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fms-compatibility -verify=cxx2b,old %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only-verify=cxx20,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@-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@-1 {{candidate constructor not viable: expects an lvalue for 1st argument}}
-};
-struct MoveOnly {
-  MoveOnly();
-  MoveOnly(MoveOnly &&);
+//in the STL when compiling with -fms-compatibility, because of issues with the
+//implementation there.
+//Feel free to delete this file when the workaround is not needed anymore.
+
+#if __INCLUDE_LEVEL__ == 0
+
+#if __cpluscplus > 202002L && __cpp_implicit_move < 202011L
+#error "__cpp_implicit_move not defined correctly"
+#endif
+
+struct nocopy {
+  nocopy(nocopy &&);
 };
-MoveOnly &();
-
-MoveOnly &(MoveOnly &) {
-  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  T &(T &) { return x; } // old-error {{cannot bind to lvalue of type}}
-template MoveOnly (MoveOnly &);
-template MoveOnly &(MoveOnly &&); // old-note {{in instantiation of function template specialization}}
-
-MoveOnly &() {
-  MoveOnly & = 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 (...) {
-}
-
-MoveOnly test6(MoveOnly x) { return x; }
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+
+namespace {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace
+
+namespace foo {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+namespace std {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace std
+} // namespace foo
+
+namespace std {
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+
+namespace {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace
+
+namespace foo {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace foo
+
+} // namespace std
+
+#include __FILE__
+
+#define SYSTEM
+#include __FILE__
+
+#elif !defined(SYSTEM)
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // 

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D105951#2894888 , @STL_MSFT wrote:

> FYI, we merged https://github.com/microsoft/STL/pull/2025 fixing the 2 
> affected `return _Istr;` occurrences in `` and `` for VS 
> 2022 17.0 Preview 4. If you find any other affected return statements, please 
> let us know ASAP (as the VS release process locks down the release branch far 
> in advance).

Thanks!

We could relax the workaround even more, and apply them just to those affected 
member functions.
That would help finding more issues, if there are any.
But at the same time we are super close to creating the release branch...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added a comment.

FYI, we merged https://github.com/microsoft/STL/pull/2025 fixing the 2 affected 
`return _Istr;` occurrences in `` and `` for VS 2022 17.0 
Preview 4. If you find any other affected return statements, please let us know 
ASAP (as the VS release process locks down the release branch far in advance).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

While we are still reviewing this and it's probably going to take longer, I 
went ahead made a DR for fixing the same issue in main: 
https://reviews.llvm.org/D106303


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 359145.
mizvekov added a comment.
Herald added a subscriber: lxfind.

Patch update:

- Fixes issue where the workaround would completely supress implicit moves.
- Improve tests to cover this issue.
- Stop applying the workaround to throw expressions, as we have not observed 
any problems there.

In D105951#2880654 , @mibintc wrote:

> Intel compiles VS2019 #include files regularly with clang, and the file 
>  compiled with -std:c++latest encounters this error report, which 
> @aaron.ballman suggests is related to this effort.
>
> I used creduce with the same command line, looking for that error diagnostic 
> with std:c++latest versus no diagnostic with std:c++20 ; this smaller test 
> case, with a few edits, is the result

Thanks for testing!

In D105951#2880920 , @aaron.ballman 
wrote:

> I think we might be in an awkward situation where some MSVC STL code needs 
> the implicit cast in order to work, such as:
> 
> (At least, Melanie's reduced example fails with this patch, but if you remove 
> the system header or namespace std bits from the test, then it passes.)

No, that was totally my fault! :-)

I did not intend for the workaround to completely disable implicit moves, just 
that it would fall back to the C++20 behavior.

The problem is that the code path for this is split around multiple places, and 
while suppressing simpler implicit moves here, I should have undone the 
suppressing of the C++20 implicit move code path elsewhere!

Sigh, I forgot it was there because I was working on another patch that 
completely removed that second code path...
Sorry for that!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaCoroutine.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
===
--- clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
+++ clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
@@ -1,50 +1,151 @@
-// 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
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only-verify=cxx2b,new %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fms-compatibility -verify=cxx2b,old %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only-verify=cxx20,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@-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@-1 {{candidate constructor not viable: expects an lvalue for 1st argument}}
-};
-struct MoveOnly {
-  MoveOnly();
-  MoveOnly(MoveOnly &&);
-};
-MoveOnly &();
-
-MoveOnly &(MoveOnly &) {
-  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  T &(T &) { return x; } // old-error {{cannot bind to lvalue of type}}
-template MoveOnly (MoveOnly &);
-template MoveOnly &(MoveOnly &&); // old-note {{in instantiation of function template specialization}}
-
-MoveOnly &() {
-  MoveOnly & = 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 (...) {
-}
+//in the STL when compiling with -fms-compatibility, because of issues with the
+//implementation there.
+//Feel free to delete this file when the workaround is not needed anymore.
+
+#if __INCLUDE_LEVEL__ == 0
+
+#if __cpluscplus > 202002L && __cpp_implicit_move < 202011L
+#error "__cpp_implicit_move not defined correctly"
+#endif
+
+struct nocopy { nocopy(nocopy&&); };
+
+int &(int &) { return x; } 

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D105951#2880654 , @mibintc wrote:

> Intel compiles VS2019 #include files regularly with clang, and the file 
>  compiled with -std:c++latest encounters this error report, which 
> @aaron.ballman suggests is related to this effort.

I think we might be in an awkward situation where some MSVC STL code needs the 
implicit cast in order to work, such as:

  In file included from c:/Program files (x86)/Microsoft Visual 
Studio/2019/Professional/VC/Tools/MSVC/14.28.29910/include\string:11:
  c:/Program files (x86)/Microsoft Visual 
Studio/2019/Professional/VC/Tools/MSVC/14.28.29910/include\xstring(4714,12): 
error: non-const lvalue reference to type 'basic_istream<...>' cannot bind to a 
temporary of type 'basic_istream<...>'
  return _Istr;
 ^

This issue was fixed with the previous workaround. But other MSVC STL code 
fails without the implicit cast, such as:

  In file included from tst_incl_filesystem.cpp:2:
  c:/Program files (x86)/Microsoft Visual 
Studio/2019/Professional/VC/Tools/MSVC/14.28.29910/include\filesystem(3099,20): 
error: call to deleted constructor of 'unique_ptr'
  return _Cleaned_link;
 ^

(At least, Melanie's reduced example fails with this patch, but if you remove 
the system header or namespace std bits from the test, then it passes.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

Intel compiles VS2019 #include files regularly with clang, and the file 
 compiled with -std:c++latest encounters this error report, which 
@aaron.ballman suggests is related to this effort.

  In file included from tst_incl_filesystem.cpp:2:
  c:/Program files (x86)/Microsoft Visual 
Studio/2019/Professional/VC/Tools/MSVC/14.28.29910/include\filesystem(3099,20): 
error: call to deleted constructor of 'unique_ptr'
  return _Cleaned_link;
 ^
  c:/Program files (x86)/Microsoft Visual 
Studio/2019/Professional/VC/Tools/MSVC/14.28.29910/include\memory(3566,5): 
note: 'unique_ptr' has been explicitly marked deleted here
  unique_ptr(const unique_ptr&) = delete;
  ^
  In file included from tst_incl_filesystem.cpp:2:
  c:/Program files (x86)/Microsoft Visual 
Studio/2019/Professional/VC/Tools/MSVC/14.28.29910/include\filesystem(3125,16): 
error: call to deleted constructor of 'unique_ptr'
  return _Cleaned_link;
 ^
  c:/Program files (x86)/Microsoft Visual 
Studio/2019/Professional/VC/Tools/MSVC/14.28.29910/include\memory(3566,5): 
note: 'unique_ptr' has been explicitly marked deleted here
  unique_ptr(const unique_ptr&) = delete;

I used creduce with the same command line, looking for that error diagnostic 
with std:c++latest versus no diagnostic with std:c++20 ; this smaller test 
case, with a few edits, is the result

  # 1 "test.cpp" 1
  # 2 "test.cpp" 3
  namespace std {
  template  struct b { static constexpr c = a; };
  template  using d = b;
  using e = d;
  template  using f = int;
  template  using g = int;
  template  constexpr am = e ::c;
  template  constexpr au = false;
  class h {
  public:
template  h(i);
template  using j = f>;
template , int, int>>> h(h &) = delete;
  };
  h k() {
h l(new wchar_t);
return l;
  }
  }
  # 22 "test.cpp" 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Matheus Izvekov via Phabricator via cfe-commits]

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

- Small simplification to implementation, don't need to cast to NamespaceDecl.
- Simplification to tests: Use some evil prep hackery to avoid needing multipe 
files.
- Verify result of getReferencedDeclOfCallee is non-null.

No tests added for the last one, as I cannot think how this could actually 
happen in this instance.
That would suggest that maybe we should assert instead, but then it seems that 
if there were
such a case, it's most likely that we would want to return false anyway.
So the assert would be basically fishing for a test case. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

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
===
--- clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
+++ clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
@@ -1,50 +1,129 @@
-// 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
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only-verify=cxx2b,new %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fms-compatibility -verify=cxx2b,old %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only-verify=cxx20,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@-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@-1 {{candidate constructor not viable: expects an lvalue for 1st argument}}
-};
-struct MoveOnly {
-  MoveOnly();
-  MoveOnly(MoveOnly &&);
-};
-MoveOnly &();
-
-MoveOnly &(MoveOnly &) {
-  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  T &(T &) { return x; } // old-error {{cannot bind to lvalue of type}}
-template MoveOnly (MoveOnly &);
-template MoveOnly &(MoveOnly &&); // old-note {{in instantiation of function template specialization}}
-
-MoveOnly &() {
-  MoveOnly & = 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 (...) {
-}
+//in the STL when compiling with -fms-compatibility, because of issues with the
+//implementation there.
+//Feel free to delete this file when the workaround is not needed anymore.
+
+#if __INCLUDE_LEVEL__ == 0
+
+#if __cpluscplus > 202002L && __cpp_implicit_move < 202011L
+#error "__cpp_implicit_move not defined correctly"
+#endif
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+
+namespace {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+} // namespace
+
+namespace foo {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+namespace std {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+} // namespace std
+} // namespace foo
+
+namespace std {
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+
+namespace {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+} // namespace
+
+namespace foo {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+} // namespace foo
+
+} // namespace std
+

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3317-3321
+  for (const DeclContext *DC = D->getDeclContext(); DC; DC = DC->getParent()) {
+if (const auto *NS = dyn_cast(DC))
+  if (NS->isStdNamespace())
+return true;
+  }

mizvekov wrote:
> aaron.ballman wrote:
> > Can you use `D->isInStdNamespace()` instead?
> It doesn't look like `isInStdNamespace` is equivalent here, even though the 
> name suggests otherwise.
> This is a small helper, all it does is:
> ```
> bool Decl::isInStdNamespace() const {
>   const DeclContext *DC = getDeclContext();
>   return DC && DC->isStdNamespace();
> }
> ```
> It helps you check `isStdNamespace` from a Decl, without having to through a 
> DeclContext yourself.
> 
> Now if we really need this workaround to apply 'recursively' like this, that 
> is a different question which I am not sure.
Good point -- `isInStdNamespace()` only looks if the declaration is in the 
`std` namespace, not whether any declaration context containing the declaration 
is in `std`. I don't have an issue with the current approach, was mostly 
thinking if there were ways to simplify it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3314
+return false;
+  const Decl *D = E.getReferencedDeclOfCallee();
+  if (!S.SourceMgr.isInSystemHeader(D->getLocation()))

aaron.ballman wrote:
> This can return `nullptr`, so we should probably return `false` in that case; 
> WDYT?
Yes I missed that, you are absolutely right!



Comment at: clang/lib/Sema/SemaStmt.cpp:3317-3321
+  for (const DeclContext *DC = D->getDeclContext(); DC; DC = DC->getParent()) {
+if (const auto *NS = dyn_cast(DC))
+  if (NS->isStdNamespace())
+return true;
+  }

aaron.ballman wrote:
> Can you use `D->isInStdNamespace()` instead?
It doesn't look like `isInStdNamespace` is equivalent here, even though the 
name suggests otherwise.
This is a small helper, all it does is:
```
bool Decl::isInStdNamespace() const {
  const DeclContext *DC = getDeclContext();
  return DC && DC->isStdNamespace();
}
```
It helps you check `isStdNamespace` from a Decl, without having to through a 
DeclContext yourself.

Now if we really need this workaround to apply 'recursively' like this, that is 
a different question which I am not sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this! We're testing the patch out internally to see if 
it solves issues for us with some of the failures we're seeing.




Comment at: clang/lib/Sema/SemaStmt.cpp:3314
+return false;
+  const Decl *D = E.getReferencedDeclOfCallee();
+  if (!S.SourceMgr.isInSystemHeader(D->getLocation()))

This can return `nullptr`, so we should probably return `false` in that case; 
WDYT?



Comment at: clang/lib/Sema/SemaStmt.cpp:3317-3321
+  for (const DeclContext *DC = D->getDeclContext(); DC; DC = DC->getParent()) {
+if (const auto *NS = dyn_cast(DC))
+  if (NS->isStdNamespace())
+return true;
+  }

Can you use `D->isInStdNamespace()` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Matheus Izvekov via Phabricator via cfe-commits]

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

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105951

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

Index: clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
===
--- clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
+++ clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
@@ -1,50 +1,48 @@
-// 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
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -I %S/Inputs/usr -isystem %S/Inputs/sys-verify=cxx2b,new %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -I %S/Inputs/usr -isystem %S/Inputs/sys -fms-compatibility -verify=cxx2b,old %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -I %S/Inputs/usr -isystem %S/Inputs/sys-verify=cxx20,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@-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@-1 {{candidate constructor not viable: expects an lvalue for 1st argument}}
-};
-struct MoveOnly {
-  MoveOnly();
-  MoveOnly(MoveOnly &&);
-};
-MoveOnly &();
-
-MoveOnly &(MoveOnly &) {
-  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  T &(T &) { return x; } // old-error {{cannot bind to lvalue of type}}
-template MoveOnly (MoveOnly &);
-template MoveOnly &(MoveOnly &&); // old-note {{in instantiation of function template specialization}}
-
-MoveOnly &() {
-  MoveOnly & = 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 (...) {
-}
+//in the STL when compiling with -fms-compatibility, because of issues with the
+//implementation there.
+//Feel free to delete this file when the workaround is not needed anymore.
+
+#if __cpluscplus > 202002L && __cpp_implicit_move < 202011L
+#error "__cpp_implicit_move not defined correctly"
+#endif
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+
+namespace {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+} // namespace
+
+namespace foo {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+namespace std {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+} // namespace std
+} // namespace foo
+
+namespace std {
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+
+namespace {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+} // namespace
+
+namespace foo {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+} // namespace foo
+
+} // namespace std
+#include 
+#include 
Index: clang/test/SemaCXX/Inputs/usr/cxx2b-p2266-disable-with-msvc-compat-usr.hpp
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/usr/cxx2b-p2266-disable-with-msvc-compat-usr.hpp
@@ -0,0 +1,33 @@
+int &(int &) { return x; } // 

[PATCH] D105951: [clang] P2266 implicit moves STL workaround

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov created this revision.
mizvekov updated this revision to Diff 358468.
mizvekov added a comment.
mizvekov updated this revision to Diff 358483.
mizvekov published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

.


mizvekov added a comment.

format.


This patch replaces the workaround for simpler implicit moves
implemented in D105518 .

The Microsoft STL currently has some issues with P2266 
.

Where before, with -fms-compatibility, we would disable simpler
implicit moves globally, with this change, we disable it only
when the returned expression is in a context contained by
std namespace and is located within a system header.

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105951

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

Index: clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
===
--- clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
+++ clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
@@ -1,50 +1,48 @@
-// 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
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -I %S/Inputs/usr -isystem %S/Inputs/sys-verify=cxx2b,new %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -I %S/Inputs/usr -isystem %S/Inputs/sys -fms-compatibility -verify=cxx2b,old %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -I %S/Inputs/usr -isystem %S/Inputs/sys-verify=cxx20,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@-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@-1 {{candidate constructor not viable: expects an lvalue for 1st argument}}
-};
-struct MoveOnly {
-  MoveOnly();
-  MoveOnly(MoveOnly &&);
-};
-MoveOnly &();
-
-MoveOnly &(MoveOnly &) {
-  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  T &(T &) { return x; } // old-error {{cannot bind to lvalue of type}}
-template MoveOnly (MoveOnly &);
-template MoveOnly &(MoveOnly &&); // old-note {{in instantiation of function template specialization}}
-
-MoveOnly &() {
-  MoveOnly & = 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 (...) {
-}
+//in the STL when compiling with -fms-compatibility, because of issues with the
+//implementation there.
+//Feel free to delete this file when the workaround is not needed anymore.
+
+#if __cpluscplus > 202002L && __cpp_implicit_move < 202011L
+#error "__cpp_implicit_move not defined correctly"
+#endif
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+
+namespace {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+} // namespace
+
+namespace foo {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+namespace std {
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+} // namespace std
+} // namespace foo
+
+namespace std {
+
+int &(int &) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int (int &) { return x; }  // cxx2b-error {{cannot bind to a temporary}}
+
+namespace {
+int &(int &) { return x; } //