[libcxx-commits] [PATCH] D121435: [libc++] Canonicalize the ranges results and their tests

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/test/std/algorithms/algorithms.results/in_out_result.pass.cpp:40
-    std::ranges::in_out_result<long, long>>);
-static_assert(std::is_convertible_v<const std::ranges::in_out_result<int, int>&&,
-    std::ranges::in_out_result<long, long>>);
----------------
Question: is it okay to remove the tests for rvalue references?


================
Comment at: libcxx/test/std/algorithms/algorithms.results/in_out_result.pass.cpp:88
   {
-    struct CopyOnly {
-      int value = 0;
----------------
I presume you find this test overkill, but just want to make sure.


================
Comment at: libcxx/test/std/algorithms/algorithms.results/no_unique_address.compile.pass.cpp:60
+#ifdef TEST_COMPILER_CLANG
+#pragma clang diagnostic ignored "-Wunknown-attributes"
+#elif defined(TEST_COMPILER_GCC)
----------------
philnik wrote:
> var-const wrote:
> > Question: why is it not possible to use `_LIBCPP_NO_UNIQUE_ADDRESS`?
> It's possible, but I think @Mordante wouldn't be very happy about it. In general we want to avoid using the libc++ macros in the test suite, and using the two attributes is completely portable (other than having warnings).
Can you add a brief comment explaining that using `_LIBCPP_NO_UNIQUE_ADDRESS` is undesirable here because of <...> so that no one gets the urge to refactor this in the future?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121435