This avoids using an libc++ internal macro in our tests.
Details
- Reviewers
philnik ldionne - Group Reviewers
Restricted Project - Commits
- rG5dc1da3e304c: [libc++][nfc] Use TEST_SAFE_STATIC.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/default.pass.cpp | ||
---|---|---|
32–33 | It looks like it might be possible to remove the if defined(_LIBCPP_VERSION) now? Also, I don't quite understand the purpose of the variable itself, but I guess that's an orthogonal question. |
libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/default.pass.cpp | ||
---|---|---|
32–33 | I haven't looked at what this is exactly used for, I just try to remove the libc++ specific dependencies from the tests. |
FWIW, I think this PR is fine as-is, but there also seems to be room for a much larger improvement to how we do (and name) _LIBCPP_SAFE_STATIC at all. Meanwhile, the point of these tests is actually to verify that the relevant ctors are constexpr, so having them use any macro that sometimes expands to "nothing" smells like it's losing us test coverage. If I were doing the followup patch (which maybe I am? let me know :)), I'd:
- Rename _LIBCPP_SAFE_STATIC to _LIBCPP_CONSTINIT
-Make _LIBCPP_CONSTINIT expand to constinit in C++20 mode, else __attribute__((__require_constant_initialization__)) if supported, else seriously consider making it a hard error (where right now it expands to nothing).
- Rename TEST_SAFE_STATIC to TEST_CONSTINIT, and make it expand to _LIBCPP_CONSTINIT if it's defined, else if C++20 then constinit, else nothing.
I think this idea sounds fine, and I if you want to clean this up even more, please do.
It looks like it might be possible to remove the if defined(_LIBCPP_VERSION) now?
Also, I don't quite understand the purpose of the variable itself, but I guess that's an orthogonal question.