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

Tue Feb 2 13:27:54 PST 2021

zibi 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.
Believe me, I'm totally with you on this but, unfortunately I have no control over visibility of `timespec64`.  We could hide this in one of the wrapper tools but this would not be fare for not being able to build library using just the repo. 

However the good news is we might remove the entire patch once we get one we get monotomic clock shipped with OS.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93542