[libcxx-commits] [PATCH] D132327: [libc++] Implement P2445R1 (`std::forward_like`)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/test/std/utilities/utility/forward_like/forward_like.msvc/test.cpp:8
+
+using namespace std;
+
----------------
I guess this is inherited from the MSVC testing style?


================
Comment at: libcxx/test/std/utilities/utility/forward_like/forward_like.msvc/test.cpp:67
+
+struct NoCtorCopyMove {
+  NoCtorCopyMove() = delete;
----------------
Are these changes in the upstream MSVC test? If not, I'd rather have them in a separate test to reduce divergence from upstream.


================
Comment at: libcxx/test/std/utilities/utility/forward_like/forward_like.msvc/test.cpp:73
+
+using NCCM = NoCtorCopyMove;
+
----------------
Why do you use an alias here? IMO it makes the test really hard to read.


================
Comment at: libcxx/test/std/utilities/utility/forward_like/forward_like.msvc/test.pass.cpp:12
+// Includes Microsoft's test that tests the entire header.
+
+#include "test.cpp"
----------------
fsb4000 wrote:
> huixie90 wrote:
> > Is this test supposed to be running on gcc/clang? I am slightly confused because it is in "msvc" folder
> Yes, all configurations should be tested.
> 
> I used https://github.com/llvm/llvm-project/tree/main/libcxx/test/std/utilities/charconv/charconv.msvc as a naming example.
> 
> I opened `gcc 12/ C++ latest` log: https://buildkite.com/llvm-project/libcxx-ci/builds/13210#0182deee-3853-484d-a646-79afa1e27420 and I found:
> 
> ```
> ... cfg.in :: std/utilities/utility/forward_like/forward_like.msvc/test.pass.cpp
>  98% [=========================================================--(B](B ETA: 00:00:04
> ```
> and `C++2b` log: https://buildkite.com/llvm-project/libcxx-ci/builds/13210#0182deee-3841-41d9-8725-06f9a249e560
> 
> ```
> ... cfg.in :: std/utilities/utility/forward_like/forward_like.msvc/test.pass.cpp
>  98% [=========================================================--(B](B ETA: 00:00:08
> ```
This way to include the test seems quite weird to me. Why can't we just name the MSVC test `test.pass.cpp`?


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

https://reviews.llvm.org/D132327