Skip to content

release/20.x: [libc++] Avoid including <features.h> on arbitrary platforms (#125587) #127310

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
Feb 19, 2025

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Feb 15, 2025

Backport cffc1ac

Requested by: @ldionne

@llvmbot llvmbot requested a review from a team as a code owner February 15, 2025 09:59
@llvmbot llvmbot added this to the LLVM 20.X Release milestone Feb 15, 2025
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 15, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Feb 15, 2025

@llvm/pr-subscribers-libcxx

Author: None (llvmbot)

Changes

Backport cffc1ac

Requested by: @ldionne


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

1 Files Affected:

  • (modified) libcxx/include/__configuration/platform.h (+2-5)
diff --git a/libcxx/include/__configuration/platform.h b/libcxx/include/__configuration/platform.h
index 2a92ce209b91f..cff99376ee24b 100644
--- a/libcxx/include/__configuration/platform.h
+++ b/libcxx/include/__configuration/platform.h
@@ -30,12 +30,9 @@
 // ... add new file formats here ...
 #endif
 
-// To detect which libc we're using
-#if __has_include(<features.h>)
+// Need to detect which libc we're using if we're on Linux.
+#if defined(__linux__) || defined(__AMDGPU__) || defined(__NVPTX__)
 #  include <features.h>
-#endif
-
-#if defined(__linux__)
 #  if defined(__GLIBC_PREREQ)
 #    define _LIBCPP_GLIBC_PREREQ(a, b) __GLIBC_PREREQ(a, b)
 #  else

@nico
Copy link
Contributor

nico commented Feb 18, 2025

(FYI, possibly causes test failures on mac: #125587 (comment))

@ldionne
Copy link
Member

ldionne commented Feb 18, 2025

Thanks for the heads up. I think we should merge this but also cherry-pick #127691 into LLVM 20 to fix that issue.

@ldionne
Copy link
Member

ldionne commented Feb 19, 2025

@tstellar Can we merge this one? I have another fix I want to cherry-pick which depends on this one.

…5587)

This partially reverts commit 5f2389d. That commit started checking
whether <features.h> was a valid include unconditionally, however codebases
are free to have such a header on their search path, which breaks compilation.
LLVM libc now provides a more standard way of getting configuration macros
like __LLVM_LIBC__.

After this patch, we only include <features.h> when we're on Linux or
when we're compiling for GPUs.

(cherry picked from commit cffc1ac)
@tstellar tstellar merged commit 876a5c9 into llvm:release/20.x Feb 19, 2025
12 of 15 checks passed
Copy link

@ldionne (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

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
Development

Successfully merging this pull request may close these issues.

4 participants