[libcxx-commits] [PATCH] D93542: [SystemZ][ZOS] Provide CLOCK_MONOTONIC alternative

Tue Feb 2 13:22:36 PST 2021

ldionne added inline comments.

================
Comment at: libcxx/CMakeLists.txt:536
+  if (ZOS)
+    add_definitions(-D_LARGE_TIME_API) # Needed for struct timespec64.
+  endif()
----------------
hubert.reinterpretcast wrote:
> ldionne wrote:
> > zibi wrote:
> > > hubert.reinterpretcast wrote:
> > > > ldionne wrote:
> > > > > Can you bury this into `gettod.h`?
> > > > That may be insufficient if the system headers with the affected interfaces get included before `gettod.h` is reached.
> > > This has to be defined here so the macro is defined before any system headers is included as stated in the comment. We took this same approach in clang code were similar macros needed to be defined with same requirements.
> > I'm really unhappy about adding random defines to make platforms behave sanely. Why doesn't z/OS provide `timespec64` when `_LARGE_TIME_API` isn't defined?
> > 
> > Also, you should be using `target_add_definitions()` here instead, to avoid polluting the build flags globally.
> It seems reasonable to provide `timespec64` through `time.h` (which provides `timespec`). However, `time.h` is a standard header, and `timespec64` does not match any of the standardized forms of identifiers that are reserved (or at least it does not clearly look like a reserved identifier to me). It is a valid design choice to use opt-in (as opposed to opt-out) macros for controlling extensions.
It's a valid design choice, but it's both inconvenient and inconsistent with what other platforms do. Anyway, this is OK but we need to use `target_add_definitions`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93542