Skip to content

[libc++] Handle Newlib and picolibc using options #152968

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

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

Conversation

inglorion
Copy link
Contributor

Previously, we relied on _NEWLIB_VERSION being defined to detect if we are building with Newlib or its derivative picolibc. This is somewhat unreliable, because whether or not NEWLIB_VERSION is defined depends on if any headers from the C library have been included or not. This could lead to inconsistent definitions, for example as observed in bug #152763. The code contained a workaround that detected picolibc by looking for its header file, with a TODO to replace this logic with a different mechanism. This change addresses that TODO by handling Newlib and picolibc similarly to how we handle musl: by providing a LIBCXX_HASLIBC option and a corresponding LIBCPP_HAS macro to be used in code.

Specifically:

  • For Newlibc, we provide LIBCXX_HAS_NEWLIB_LIBC and _LIBCPP_HAS_NEWLIB_LIBC.

  • For picolibc, we provide LIBCXX_HAS_PICOLIBC and _LIBCPP_HAS_PICOLIBC.

Existing code has been rewritten so that instead of checking for defined(_NEWLIB_VERSION) it checks for
_LIBCPP_HAS_NEWLIB_LIBC || _LIBCPP_HAS_PICOLIBC.

Previously, we relied on _NEWLIB_VERSION being defined to detect if we
are building with Newlib or its derivative picolibc. This is somewhat
unreliable, because whether or not _NEWLIB_VERSION is defined depends
on if any headers from the C library have been included or not. This
could lead to inconsistent definitions, for example as observed in
bug llvm#152763. The code contained a workaround that detected picolibc by
looking for its header file, with a TODO to replace this logic with
a different mechanism. This change addresses that TODO by handling
Newlib and picolibc similarly to how we handle musl: by providing a
LIBCXX_HAS_*LIBC option and a corresponding _LIBCPP_HAS_* macro to be
used in code.

Specifically:

 - For Newlibc, we provide LIBCXX_HAS_NEWLIB_LIBC and
   _LIBCPP_HAS_NEWLIB_LIBC.

 - For picolibc, we provide LIBCXX_HAS_PICOLIBC and
   _LIBCPP_HAS_PICOLIBC.

Existing code has been rewritten so that instead of checking for
defined(_NEWLIB_VERSION) it checks for
_LIBCPP_HAS_NEWLIB_LIBC || _LIBCPP_HAS_PICOLIBC.
@inglorion
Copy link
Contributor Author

Some open questions:

  1. This now requires an option to be set. We can probably autodetect the correct value. Should we? How would we implement that?
  2. Instead of having boolean options for various libcs, should we have a single option to select the libc flavor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant