This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Prefer __has_builtin for detecting compiler-provided type_traits
ClosedPublic

Authored by philnik on Jul 3 2022, 6:30 PM.

Details

Summary

Both clang and GCC support using __has_builtin for detecting compiler-provided type_traits. Use it instead of __has_keyword or __has_feature to remove special-casing for GCC-provided builtins

Diff Detail

Event Timeline

philnik created this revision.Jul 3 2022, 6:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 3 2022, 6:30 PM
philnik requested review of this revision.Jul 3 2022, 6:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 3 2022, 6:30 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

In general looks good, but I really like to see it pass all CIs. (I know the current failures aren't due to this patch.)

libcxx/include/__type_traits/is_convertible.h
32

please update the comment to match the #if, same for the #endif.

ldionne accepted this revision.Jul 5 2022, 2:03 PM

LGTM, but I don't understand what's the special-casing for GCC that you mention in the commit message. Can you elaborate?

This revision is now accepted and ready to land.Jul 5 2022, 2:03 PM

LGTM, but I don't understand what's the special-casing for GCC that you mention in the commit message. Can you elaborate?

We've had a few __has_keyword(whatever) || _LIBCPP_COMPILER_GCC before (for example in is_trivially_destructible.h). The || _LIBCPP_COMPILER_GCC is now unnecessary, since GCC correctly reports support for builtin type traits with __has_builtin.

This revision was landed with ongoing or failed builds.Jul 6 2022, 4:34 AM
This revision was automatically updated to reflect the committed changes.
Mordante added inline comments.Jul 10 2022, 5:10 AM
libcxx/include/__type_traits/is_convertible.h
32

I've addressed this comment myself.