[libcxx-commits] [PATCH] D154246: [libc++] Add check for building with picolibc

[Alexander Richardson via Phabricator via libcxx-commits]

arichardson added inline comments.


================
Comment at: libcxx/test/configs/armv7m-libc++.cfg.in:17
+    # which for picolibc is defined in picolibc.h.
+    ' -include picolibc.h'
+))
----------------
michaelplatings wrote:
> arichardson wrote:
> > Doesn't this mean the library can't be used without adding this include to any build system that is trying to use libc++ with picolibc? I think it would be a lot cleaner if we could just add the required includes to the libc++ source code?
> > 
> > As far as I recall from the last time I tried to use libc++ with picolibc this was not needed but I may have been building a slightly different configuration
> > Doesn't this mean the library can't be used without adding this include to any build system that is trying to use libc++ with picolibc?
> 
> Most libc++ headers already indirectly include picolibc.h so most of the time this isn't a problem in practice. It's a minority of tests that fail without this include.
> 
> > I think it would be a lot cleaner if we could just add the required includes to the libc++ source code?
> 
> I agree with you that it would be cleaner to remove this line and instead change the libc++ headers. However I can imagine that adding extra includes could cause undesirable knock-on effects. I think it's best to limit the scope of this change to only adding the test setup and not touching libc++ itself.
Do you have a list of tests that fail? If it's just a few maybe the better solution is to add the fixme to REQUIRES?


================
Comment at: libcxx/test/configs/armv7m-libc++.cfg.in:38
+# using this feature.
+config.available_features.add('LIBCXX-PICOLIBC-FIXME')
+
----------------
michaelplatings wrote:
> arichardson wrote:
> > Could this be autodetected instead by checking for the presence of the picolibc header? Then it would also work out of the box with the default config file.
> I don't think we can infer that we're targeting picolibc from the presence of the header. It might also happen to be on an include path in a scenario in which another platform is targeted. I think it's best to be explicit here.
I meant trying to compile something like the following:
```
#include <stdio.h>
#ifndef __PICOLIBC__
#error not picolibc
#endif
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154246