[libcxx-commits] [PATCH] D89040: [libcxx] Allow checking for newlib without using _NEWLIB_VERSION.

[Hafiz Abid Qadeer via Phabricator via libcxx-commits]

abidh added a comment.

In D89040#2344267 <https://reviews.llvm.org/D89040#2344267>, @ldionne wrote:

> 

Thanks for the review.

> I see a couple issues with this:
>
> Actually why do we even need `_LIBCPP_HAS_NEWLIB_LIBC`?

Consider example of __config, it already has some code conditional on _NEWLIB_VERSION. But this macro will always be undefined in this file as nothing is bringing in its definition. Note that _NEWLIB_VERSION is not defined by the compiler but it needs newlib.h to be included either directly or indirectly through some other library header.  This is what _LIBCPP_HAS_NEWLIB_LIBC was trying to do. Detect newlib where we could not use _NEWLIB_VERSION.

> 1. I don't want to add a new CMake option just to detect an underlying libc. We should be able to detect it through other means.

One alternate I could think of what to use -include newlib.h in my compiler flags. That works but the problem with nasty_macros.h still remains unless I use an override of configure_compile_flags_header_includes in my customized config file to skip the check for nasty_macros.h. This can work but it will require all newlib users to handle these things in their own config files.

> 2. We should have testers for this configuration. Newlib is not currently supported (well.. not officially at least), and we need some kind of testing if we pretend we're supporting it.

You mentioned in https://reviews.llvm.org/D88825 that you will write some instruction.  I will be happy to setup one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89040/new/

https://reviews.llvm.org/D89040