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

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:732
+    Builder.defineMacro("_OPEN_DEFAULT");
+    Builder.defineMacro("_UNIX03_WITHDRAWN");
+    Builder.defineMacro("__370__");
----------------
This is not defined by z/OS XL C/C++. It seems that this is more of a macro to be defined by a user application (perhaps as part of its configuration/port for z/OS) and less a macro that should be predefined by the compiler.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:750
+
+    if (Opts.C11 || Opts.GNUMode)
+      Builder.defineMacro("__IBM_UTF_LITERAL");
----------------
Shouldn't UTF literals be reported as being enabled under strict C++11?


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:753
+
+    if (Opts.C11 || (Opts.GNUMode && !Opts.CPlusPlus))
+      Builder.defineMacro("__IBMC_GENERIC");
----------------
Is this consistent with `__has_extension` with respect to `-pedantic-errors`?
That is, `-pedantic-errors` causes `__has_extension` to report the value that `__has_feature` would report. Compiler Explorer link: https://godbolt.org/z/EEn8rr

Same question for all of the other IBM-style feature test macros.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:758
+      Builder.defineMacro("__DLL__");
+      // Macro __wchar_t exposes the definition of wchar_t data type
+      // in system headers.
----------------
Should the comment instead say that `__wchar_t` should be defined so that the system headers do not try to declare `wchar_t` as a typedef?


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:761
+      Builder.defineMacro("__wchar_t");
+      Builder.defineMacro("_XOPEN_SOURCE", "600");
+    }
----------------
Same comment as before re: macros that should be declared by the application.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:764
+
+    if (Opts.C11 || Opts.CPlusPlus11 || Opts.GNUMode) 
+      Builder.defineMacro("__IBMC_NORETURN");
----------------
I don't see the relation between C++11 and `_Noreturn`. It's an extension in C++ that's available under, e.g., `-std=c++03`.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:767
+
+    if (Opts.C11 || (Opts.GNUMode && Opts.CPlusPlus)) {
+      Builder.defineMacro("__IBM_CHAR16_T__");
----------------
`char16_t` is not a keyword in C11, so `__IBM_CHAR16_T__` should not be defined for C. Same re: `char32_t`.

Also, `char16_t` and `char32_t` are indeed keywords in C++11.


================
Comment at: clang/lib/Basic/Targets/OSTargets.h:770
+      Builder.defineMacro("__IBM_CHAR32_T__");
+      Builder.defineMacro("__IBMCPP_UTF_LITERAL__");
+    }
----------------
The "IBMCPP" macro should not be defined in C modes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324