[libcxx-commits] [PATCH] D99641: [libcxx] [test] Fix invocable tests on Windows

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/concepts/concepts.callable/concept.invocable/invocable.compile.pass.cpp:232-237
+#ifndef _MSC_VER
+// With MSVC 19.28 and newer, this test passes. However Clang currently only
+// mimics the old MSVC behaviour where this assert fails, regardless of what
+// version of MSVC it targets.
 static_assert(!std::invocable<multiple_overloads&, long>);
+#endif
----------------
curdeius wrote:
> So, if we wanted to be precise (and if clang-cl correctly mimicked 19.28+ behaviour), it would be then `#if !(defined(_MSC_VER) && defined(__clang__)) || !(defined(_MSC_VER) && (_MSC_VER < 1928))`, right?
> 
> So to:
> 1) turn it off for clang-cl
> 2) turn it off for MSVC before 19.28
> 
> But I'm not sure we want that. Just noting.
This is becoming insane. `libcxx/test/std/concepts/` is not the right place to be testing MSVC compatibility features in clang-cl's overload resolution logic. Take it to `clang/test/`, please — and take the suggested edit here.


================
Comment at: libcxx/test/std/concepts/concepts.callable/concept.invocable/invocable.compile.pass.cpp:243-245
+#ifndef _MSC_VER
 static_assert(!std::invocable<multiple_overloads&, long&>);
+#endif
----------------
Take the suggested edit, please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99641