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

Thu May 20 22:17:20 PDT 2021

cjdb 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>());
----------------
zoecarver wrote:
> 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?
> Do we still support GCC 10?

We only support GCC 10.

> And when it does, can/should we remove all this `UNSUPPORTED` cruft?

Why? It's not hurting anything to keep it there.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102119