[libcxx-commits] [PATCH] D118503: [SystemZ][z/OS] Add ASCII and 32-bit variants for libc++.

[Sean via Phabricator via libcxx-commits]

SeanP added inline comments.


================
Comment at: libcxx/include/__config_site.in:13
 #cmakedefine _LIBCPP_ABI_VERSION @_LIBCPP_ABI_VERSION@
+#if defined(__MVS__)
+#include <features.h> // for __NATIVE_ASCII_F
----------------
ldionne wrote:
> We don't put any logic inside `__config_site`. This logic should instead be implemented by setting a different namespace in the cache you use when building for EBCDIC/ASCII (you probably want two caches).
Unless we are missing something, I don't think this kind of requirement is reasonable.  We need to be able to the distinguish between ebcdic/ascii with the installed headers.  If we don't put conditional logic into the `__config` header, we will be creating horribly complicated code (if it's even possible) in the compiler.  If we can't put conditional logic into `__config`, we will be required to ship two copies of the the libc++ headers (one where `__config` is set up for ascii and one where it is set up for ebcdic).  This is going to force the compiler to change the include dir for libc++ based on the the ascii/ebcdic option.  That is going to break the -isystem option.

We need one set of header files that defines `_LIBCPP_ABI_NAMESPACE` differently at compile time depending on if it is ascii or ebcdic.  This is the only way we can see doing this.  If there is a better way we are open to doing it that way.  The cache files don't work as they would result in two different set of headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118503