[libcxx-commits] [PATCH] D102119: [libcxx][optional] adds missing constexpr operations

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/test/std/utilities/optional/optional.object/optional.object.assign/emplace.pass.cpp:233
         test_one_arg<const T>();
+#if TEST_STD_VER >= 20 && !defined(TEST_COMPILER_GCC)
+        static_assert(test_one_arg<T>());
----------------
cjdb wrote:
> ldionne wrote:
> > This should be:
> > 
> > ```
> > #if TEST_STD_VER > 20 || (TEST_STD_VER >= 20 && defined(_LIBCPP_VER))
> > ```
> > 
> > Basically, test it in C++2b, or in C++20 but only when testing libc++, since it is out own extension. Otherwise, a conforming implementation will fail the test suite in C++20 mode because they don't implement `constexpr` for `std::optional`. This applies everywhere.
> > 
> > Also, what's the deal with GCC? If it fails those tests, we should first report the issue to GCC and then use some `XFAIL` markup to disable it instead.
> > Also, what's the deal with GCC? If it fails those tests, we should first report the issue to GCC and then use some `XFAIL` markup to disable it instead.
> 
> GCC 10 doesn't like our constexpr `construct_at` (GCC 11 is okay with it). I wasn't comfortable saying GCC couldn't test this file //at all// given it still works outside of constant expressions.
I think you should mark the whole file as `UNSUPPORTED: gcc-10`. IMO it's better to test everything on one version of GCC than some things on no versions of GCC. 

Do we still support GCC 10? When does that change go through? And when it does, can/should we remove all this `UNSUPPORTED` cruft?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102119