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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
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)
----------------
var-const wrote:
> 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?
I indeed dislike `_LIBCPP_NO_UNIQUE_ADDRESS` in the tests. Our tests in `std` should (in theory) work with all Standard library implementations. Using `_LIBCPP` specific macros make these tests less portable. I know MSVC STL uses our tests, so it's not a purely hypothetical. However I think it makes sense to make a specific test macro for this case, for example `TEST_UNIQUE_ADDRESS` That macro can then hide we need different attributes for MSVC and Clang. Something along the lines
```
#if TEST_STD_VER >= 17
# if defined(_WIN32) // This might require tuning, not sure what Clang on Windows does.
#   define TEST_UNIQUE_ADDRESS [[msvc::no_unique_address]]
# else
#   define TEST_UNIQUE_ADDRESS [[no_unique_address]]
# endif
#else
# define TEST_UNIQUE_ADDRESS
#endif
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121435