Skip to content

[libc++] Remove a few __has_foo defines in __config #90511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 2, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Apr 29, 2024

All the compilers we support implement those macros or builtins, so it's not useful to have a fallback to 0 when they're not implemented.

@ldionne ldionne requested a review from a team as a code owner April 29, 2024 18:38
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 29, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

All the compilers we support implement those macros or builtins, so it's not useful to have a fallback to 0 when they're not implemented.


Full diff: https://github.com/llvm/llvm-project/pull/90511.diff

1 Files Affected:

  • (modified) libcxx/include/__config (-24)
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 97cdd020c55d1f..ed55be2eaa76a1 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -334,26 +334,6 @@ _LIBCPP_HARDENING_MODE_DEBUG
 #    define _LIBCPP_CXX03_LANG
 #  endif
 
-#  ifndef __has_attribute
-#    define __has_attribute(__x) 0
-#  endif
-
-#  ifndef __has_builtin
-#    define __has_builtin(__x) 0
-#  endif
-
-#  ifndef __has_extension
-#    define __has_extension(__x) 0
-#  endif
-
-#  ifndef __has_feature
-#    define __has_feature(__x) 0
-#  endif
-
-#  ifndef __has_cpp_attribute
-#    define __has_cpp_attribute(__x) 0
-#  endif
-
 #  ifndef __has_constexpr_builtin
 #    define __has_constexpr_builtin(x) 0
 #  endif
@@ -375,10 +355,6 @@ _LIBCPP_HARDENING_MODE_DEBUG
 
 #  define __has_keyword(__x) !(__is_identifier(__x))
 
-#  ifndef __has_include
-#    define __has_include(...) 0
-#  endif
-
 #  ifndef __has_warning
 #    define __has_warning(...) 0
 #  endif

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with __has_feature and __has_extension added (until we switch to GCC 14). Maybe add a TODO for that.

Comment on lines 345 to 351
# ifndef __has_extension
# define __has_extension(__x) 0
# endif

# ifndef __has_feature
# define __has_feature(__x) 0
# endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These aren't available in GCC until 14.

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with __has_feature and __has_extension added (until we switch to GCC 14). Maybe add a TODO for that.

FYI GCC 14 is planned for next week. So we might be able to land this review quite soon. (Assuming there are no GCC 14 migration issues.)

LGTM!

All the compilers we support implement those macros or builtins,
so it's not useful to have a fallback to 0 when they're not implemented.
@ldionne ldionne force-pushed the review/config-simplify-__has_foo branch from 7e9885a to 66483e0 Compare May 1, 2024 16:25
@philnik777
Copy link
Contributor

LGTM with __has_feature and __has_extension added (until we switch to GCC 14). Maybe add a TODO for that.

FYI GCC 14 is planned for next week. So we might be able to land this review quite soon. (Assuming there are no GCC 14 migration issues.)

Where did you find that info?

@mordante
Copy link
Member

mordante commented May 1, 2024

LGTM with __has_feature and __has_extension added (until we switch to GCC 14). Maybe add a TODO for that.

FYI GCC 14 is planned for next week. So we might be able to land this review quite soon. (Assuming there are no GCC 14 migration issues.)

Where did you find that info?

https://gcc.gnu.org/pipermail/gcc/2024-April/243823.html

@ldionne ldionne merged commit 46a5de6 into llvm:main May 2, 2024
@ldionne ldionne deleted the review/config-simplify-__has_foo branch May 2, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants