[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan added inline comments.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:730
+                    MacroBuilder &Builder) const override {
+    Builder.defineMacro("_LONG_LONG");
+    Builder.defineMacro("_OPEN_DEFAULT");
----------------
hubert.reinterpretcast wrote:
> The comment from https://reviews.llvm.org/D85324?id=283290#inline-786609 applies here as well. `_LONG_LONG` should not be defined under `-std=c89 -pedantic-errors` or `-std=c89 -Werror=long-long`.
I can add a FIXME here similar to what AIX did. 

```
//FIXME: LONG_LONG should not be defined under -std=c89
```
Let me know if there is a better solution.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:755
+      // is not declared as a typedef in system headers.
+      Builder.defineMacro("__wchar_t");
+      // XOPEN_SOURCE=600 is required to build libcxx.
----------------
hubert.reinterpretcast wrote:
> The corresponding AIX code checks for `-Xclang -fno-wchar`. The behaviour of `stddef.h` differs when using `-fno-wchar` between AIX and Linux though. Linux suppresses the typedef based on `__cplusplus`. Arguably, the OS header difference should not really factor into whether the compiler defines a macro that indicates the presence of a `wchar_t` fundamental type.
Thanks, I will add the same guard that AIX uses.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:766
+    if (Opts.C11 || Opts.GNUMode) {
+      Builder.defineMacro("__IBM_UTF_LITERAL");
+      Builder.defineMacro("__IBMC_NORETURN");
----------------
hubert.reinterpretcast wrote:
> The GNU extension modes do not cause u-prefixed, etc. string literals to be accepted where the base language level would treat the prefix as a separate identifier. Also noting here that the previous comment from https://reviews.llvm.org/D85324?id=283290#inline-786628 was made based on noting that `__IBM_UTF_LITERAL` is defined by the XL compiler in the appropriate C++ modes.
We've updated the system headers so that we no longer need to define these macros.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:767
+      Builder.defineMacro("__IBM_UTF_LITERAL");
+      Builder.defineMacro("__IBMC_NORETURN");
+    }
----------------
hubert.reinterpretcast wrote:
> I would expect that the Clang implementation of the "IBM-style" feature test macros would behave similarly to the native feature testing in Clang. That is, `__IBMC_NORETURN` is defined when `_Noreturn` is available as an extension. `_Noreturn` is available as an "orthogonal" or "conforming" extension in modes such as `-std=c89` and `-std=c++03`. The extension is disabled via warnings-as-errors.
Right, we are able to remove this macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324