Skip to content

[libc++] Check for _newlib_version.h instead of picolibc.h #152766

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

inglorion
Copy link
Contributor

Fixes #152763.

#131921 added some code to pull in a definition of _NEWLIB_VERSION if it exists. It does this by checking __has_include(<picolibc.h>) and including it if so. However, this does not work for systems that have newlib rather than picolibc. This change instead checks for and includes _newlib_version.h, which should work for both newlib and picolibc.

@inglorion inglorion requested a review from a team as a code owner August 8, 2025 17:23
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Aug 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 8, 2025

@llvm/pr-subscribers-libcxx

Author: None (inglorion)

Changes

Fixes #152763.

#131921 added some code to pull in a definition of _NEWLIB_VERSION if it exists. It does this by checking __has_include(<picolibc.h>) and including it if so. However, this does not work for systems that have newlib rather than picolibc. This change instead checks for and includes _newlib_version.h, which should work for both newlib and picolibc.


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

1 Files Affected:

  • (modified) libcxx/include/__configuration/platform.h (+2-2)
diff --git a/libcxx/include/__configuration/platform.h b/libcxx/include/__configuration/platform.h
index f3c199dee172b..c14089b5fc074 100644
--- a/libcxx/include/__configuration/platform.h
+++ b/libcxx/include/__configuration/platform.h
@@ -45,8 +45,8 @@
 // This is required in order for _NEWLIB_VERSION to be defined in places where we use it.
 // TODO: We shouldn't be including arbitrarily-named headers from libc++ since this can break valid
 //       user code. Move code paths that need _NEWLIB_VERSION to another customization mechanism.
-#if __has_include(<picolibc.h>)
-#  include <picolibc.h>
+#if __has_include(<_newlib_version.h>)
+#  include <_newlib_version.h>
 #endif
 
 #ifndef __BYTE_ORDER__

Fixes llvm#152763.

llvm#131921 added some code to
pull in a definition of _NEWLIB_VERSION if it exists. It does this
by checking __has_include(<picolibc.h>) and including it if so.
However, this does not work for systems that have newlib rather
than picolibc. With this change, we check for either picolibc.h
or _newlib_version.h, which works for both newlib and picolibc.
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.

We're trying to not include arbitrary headers anymore, since that caused problems before. See #147956, which should also fix your issue.

@inglorion
Copy link
Contributor Author

Thanks for pointing me at #147956. I had been working on something similar in #152968, but I guess I can abandon that.

Any idea if we are planning to merge #147956 soon? If so, no need to take this quick fix. If it's going to take a while, we might consider taking this in the interim.

@philnik777
Copy link
Contributor

I really don't know when it'll land. IIRC the initial reason we're doing it this way was that there were problems with just including some header though, so I'm not sure an intermittent solution will work.

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
3 participants