This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Remove _LIBCPP_HAS_NO_STRONG_ENUMS.
ClosedPublic

Authored by Mordante on Feb 15 2022, 10:20 AM.

Details

Summary

All supported compilers have implemented this feature.
Therefore use the language version instead of the feature macro.

Diff Detail

Event Timeline

Mordante requested review of this revision.Feb 15 2022, 10:20 AM
Mordante created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2022, 10:20 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik accepted this revision as: philnik.Feb 15 2022, 10:22 AM
ldionne requested changes to this revision.Feb 15 2022, 11:12 AM
ldionne added a subscriber: ldionne.

I think those should be guarded by a standard version check instead, no?

This revision now requires changes to proceed.Feb 15 2022, 11:12 AM

+1 what Louis said; I was gonna say the same thing but he beat me to it. :) I doubt we support any compilers without cxx_strong_enums anymore, right?

Good point. It also seems Clang only supports it in C++11 and not in C++03.
In that case I prefer to remove _LIBCPP_HAS_NO_STRONG_ENUMS from the codebase and test for C++03 instead.
That the _LIBCPP_DECLARE_STRONG_ENUM macros will be gated by the language version.
Agreed on this approach?

Good point. It also seems Clang only supports it in C++11 and not in C++03.
In that case I prefer to remove _LIBCPP_HAS_NO_STRONG_ENUMS from the codebase and test for C++03 instead.
That the _LIBCPP_DECLARE_STRONG_ENUM macros will be gated by the language version.
Agreed on this approach?

SGTM!
(I wondered if the _LIBCPP_DECLARE_STRONG_ENUM macro could be completely eliminated, but no, I see that we do have at least one use, in <ios>, that needs to keep working in C++03 mode. And also in <future> for some reason; and maybe all its uses.)

To clarify in case it's needed: The guard around define _LIBCPP_DECLARE_STRONG_ENUM(x) should now use #if defined(_LIBCPP_CXX03_LANG). The macro _LIBCPP_HAS_NO_STRONG_ENUMS should vanish.

libcxx/test/std/utilities/meta/meta.trans/meta.trans.sign/make_unsigned.pass.cpp
29

To clarify in case it's needed: This should now use TEST_STD_VER >= 11.

To clarify in case it's needed: The guard around define _LIBCPP_DECLARE_STRONG_ENUM(x) should now use #if defined(_LIBCPP_CXX03_LANG). The macro _LIBCPP_HAS_NO_STRONG_ENUMS should vanish.

Correct that is the plan. So I revert this patch locally and grep for _LIBCPP_HAS_NO_STRONG_ENUMS, replace it with the proper version macro and reuse this review for the new approach.

Mordante updated this revision to Diff 411162.Feb 24 2022, 9:25 AM

Use the language version instead of the feature test.

Mordante retitled this revision from [libc++][nfc] Add TEST_HAS_NO_STRONG_ENUMS. to [libc++] Remove _LIBCPP_HAS_NO_STRONG_ENUMS..Feb 24 2022, 9:27 AM
Mordante edited the summary of this revision. (Show Details)
Quuxplusone accepted this revision.Feb 24 2022, 5:20 PM

Feel free to poke @ldionne and wait for his approval, but IMNSHO this patch now conforms to everything Louis was asking for. :) Thanks for doing this!

Mordante updated this revision to Diff 411822.Feb 28 2022, 9:25 AM

Rebased to see test whether CI passes.

ldionne accepted this revision.Mar 1 2022, 11:19 AM

Thanks!

This revision is now accepted and ready to land.Mar 1 2022, 11:19 AM
This revision was automatically updated to reflect the committed changes.