Page MenuHomePhabricator

[libc++] Guard __cpp_lib_constexpr_dynamic_alloc with compiler support
AcceptedPublic

Authored by arichardson on Nov 10 2020, 5:44 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Summary

This fix test failures in std/utilities/memory with Clang 8. Clang 8
supports the -std=c++2a flag, but does not implement
__cpp_constexpr_dynamic_alloc, so we get errors when evaluating some static
asserts in these test files.

I am not sure if the library version macro needs to depend on compiler
support, but some of the other macros already do this.

Diff Detail

Event Timeline

arichardson created this revision.Nov 10 2020, 5:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 10 2020, 5:44 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
arichardson requested review of this revision.Nov 10 2020, 5:44 AM
arichardson updated this revision to Diff 304159.
ldionne requested changes to this revision.Nov 20 2020, 12:04 PM

Thanks for looking into this. I just recently hacked up something to silence these failures on the Clang 8 builders, but I think this is better.

However, the tests like libcxx/test/std/language.support/support.limits/support.limits.general/memory.version.pass.cpp are auto-generated. You need to modify libcxx/utils/generate_feature_test_macro_components.py instead.

This revision now requires changes to proceed.Nov 20 2020, 12:04 PM

Thanks for looking into this. I just recently hacked up something to silence these failures on the Clang 8 builders, but I think this is better.

However, the tests like libcxx/test/std/language.support/support.limits/support.limits.general/memory.version.pass.cpp are auto-generated. You need to modify libcxx/utils/generate_feature_test_macro_components.py instead.

Will do. I clearly can't read, it even says so in the first few lines of that file...

  • update the script instead
ldionne requested changes to this revision.Dec 1 2020, 2:20 PM
ldionne added inline comments.
libcxx/test/std/utilities/memory/allocator.traits/allocator.traits.members/construct.pass.cpp
175 ↗(On Diff #308091)

Same as below.

libcxx/test/std/utilities/memory/allocator.traits/allocator.traits.members/destroy.pass.cpp
134

I'd much rather keep this conditioned on TEST_STD_VER. Otherwise, we're assuming that __cpp_constexpr_dynamic_alloc is defined properly, else this will never get tested.

This shouldn't be an issue since the test is disabled with older Clangs anyway.

libcxx/test/std/utilities/memory/default.allocator/allocator.members/allocate.pass.cpp
83 ↗(On Diff #308091)

Same.

This revision now requires changes to proceed.Dec 1 2020, 2:20 PM

rebase

libcxx/utils/generate_feature_test_macro_components.py
1

I made this change because running the script failed with python2.7

arichardson marked 2 inline comments as done.Jan 8 2021, 4:49 AM
ldionne accepted this revision.Jan 8 2021, 7:57 AM

I'm not a big fan of making __cpp_lib_constexpr_dynamic_alloc depend on __cpp_constexpr_dynamic_alloc. I'd rather we assume the compiler provides the right support when C++20 is enabled, since this simplifies everything. But since we currently claim to support older compilers, let's make this change.

This revision is now accepted and ready to land.Jan 8 2021, 7:57 AM