[libcxx-commits] [PATCH] D131315: [libc++] Implement P2273R3 (`constexpr` `unique_ptr`)

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

While checking I missed several tests, but it seems you mentioned that.

In D131315#3704130 <https://reviews.llvm.org/D131315#3704130>, @fsb4000 wrote:

> I added some constexpr tests.
>
> But many tests are not constexpr friendly and I don't know how to fix them :(
>
> Not full list:
>
> /unique.ptr.class/unique.ptr.modifiers/reset.pass.cpp
>
> They use static variables and sometimes global variables...

I have looked at a solution and for the reset test I've created a fix. This basically removes the parts that can't be tested in a `constexpr` way; in this case a `static` class member. So this means our coverage during constant evaluation is a bit lower, but we can still test the basics. (We've done this in other tests too.) Maybe some day when the `constexpr` restrictions will be reduced further we can then re-enable these tests.

I've tested my patch locally with all supported language standards, feel free to use this in your patch.
http://paste.debian.net/1250391/

(I want to validate this patch against the paper when all tests are done.)



================
Comment at: libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/constexpr.pass.cpp:9
+
+// Copyright (c) Microsoft Corporation.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
----------------
I would propose a different way to include this test, in a similar fashion we did with the charconv tests
- make a subdirectory p2273r3.msvc
- rename this file to test.compile.cpp and remove all libc++ specifics (except for the overwrite when it doesn't compile) like the EDG work-arounds
- add a test.compile.pass.cpp that has the libc++ specific which includes `test.compile.cpp`, in a similar fashion as `like test/std/utilities/charconv/charconv.msvc/test.pass.cpp`

This means the diff between us and MSVC STL remains minimal. (I still want to look at using more MSVC STL tests in the future, but that's very low on my priority list.)


================
Comment at: libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/constexpr.pass.cpp:142
+
+int main() {} // COMPILE-ONLY
----------------
I wonder whether that's needed, but I'll ask the MSVC STL team, we don't use it for compile-time only tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131315