abhina.sreeskantharajan marked 2 inline comments as done.
abhina.sreeskantharajan added inline comments.
================
Comment at: clang/lib/Basic/Targets/OSTargets.h:732
+ Builder.defineMacro("_OPEN_DEFAULT");
+ Builder.defineMacro("_UNIX03_WITHDRAWN");
+ Builder.defineMacro("__370__");
----------------
hubert.reinterpretcast wrote:
> 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.
This is required for building libcxx, I'll add a comment here.
================
Comment at: clang/lib/Basic/Targets/OSTargets.h:750
+
+ if (Opts.C11 || Opts.GNUMode)
+ Builder.defineMacro("__IBM_UTF_LITERAL");
----------------
hubert.reinterpretcast wrote:
> Shouldn't UTF literals be reported as being enabled under strict C++11?
I will get back to you on this. I will need to investigate whether we are
keeping this macro.
================
Comment at: clang/lib/Basic/Targets/OSTargets.h:753
+
+ if (Opts.C11 || (Opts.GNUMode && !Opts.CPlusPlus))
+ Builder.defineMacro("__IBMC_GENERIC");
----------------
hubert.reinterpretcast wrote:
> 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.
This macro is unused, so we are able to remove this.
================
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.
----------------
hubert.reinterpretcast wrote:
> 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?
You're right. I'll update the comment to be more accurate.
================
Comment at: clang/lib/Basic/Targets/OSTargets.h:761
+ Builder.defineMacro("__wchar_t");
+ Builder.defineMacro("_XOPEN_SOURCE", "600");
+ }
----------------
hubert.reinterpretcast wrote:
> Same comment as before re: macros that should be declared by the application.
Same as above: This is required for building libcxx.
================
Comment at: clang/lib/Basic/Targets/OSTargets.h:764
+
+ if (Opts.C11 || Opts.CPlusPlus11 || Opts.GNUMode)
+ Builder.defineMacro("__IBMC_NORETURN");
----------------
hubert.reinterpretcast wrote:
> 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`.
Thanks, I will remove CPlusPlus11.
================
Comment at: clang/lib/Basic/Targets/OSTargets.h:767
+
+ if (Opts.C11 || (Opts.GNUMode && Opts.CPlusPlus)) {
+ Builder.defineMacro("__IBM_CHAR16_T__");
----------------
hubert.reinterpretcast wrote:
> `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.
Right, these seem to only require C++, so I will update the check. But I will
also follow up on whether we plan to keep these macros here.
================
Comment at: clang/lib/Basic/Targets/OSTargets.h:770
+ Builder.defineMacro("__IBM_CHAR32_T__");
+ Builder.defineMacro("__IBMCPP_UTF_LITERAL__");
+ }
----------------
hubert.reinterpretcast wrote:
> The "IBMCPP" macro should not be defined in C modes.
See above: I will update this, but continue to investigate whether we need this
macro.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85324/new/
https://reviews.llvm.org/D85324
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
