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

[Michael Platings via Phabricator via libcxx-commits]

michaelplatings marked 5 inline comments as done.
michaelplatings added inline comments.


================
Comment at: libcxx/test/configs/armv7m-libc++.cfg.in:12
+    '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support'
+    ' -include picolibc.h'
+))
----------------
ldionne wrote:
> Why is that include needed?
Added comment explaining it.


================
Comment at: libcxx/test/libcxx/system_reserved_names.gen.py:105
+// picolibc uses __input as a parameter name of a64l & l64a
+#ifndef _NEWLIB_VERSION
+# define __input SYSTEM_RESERVED_NAME
----------------
ldionne wrote:
> What is the relationship between picolibc and `_NEWLIB_VERSION`?
Picolibc is derived from Newlib, so defines _NEWLIB_VERSION. I've updated the comment to make it clear that it applies to both libraries.


================
Comment at: libcxx/utils/ci/run-buildbot:639
+        --install-dir "${INSTALL_DIR}" \
+        --target armv7m-none-eabi
+
----------------
ldionne wrote:
> It's weird that you're passing this here but also hardcoding it in your `Armv7M-picolibc.cmake` cache file. IMO it would make more sense *not* to hardcode it in the cache.
Yes I'd prefer not to put the target in the CMake cache file, but that I think that would cause confusion because it would be inconsistent with all the other CMake cache files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154246