[libcxx-commits] [PATCH] D79305: chrono: check _POSIX_TIMERS before using clock_gettime

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a subscriber: EricWF.
ldionne added a comment.
This revision now requires changes to proceed.

Please make a corresponding change at `libcxx/src/filesystem/operations.cpp:40`, where the `#if` has been copied.

@EricWF What would you say about having a header in `src/` that contains these additional configuration checks only used for building the dylib? Either that, or we should move these checks to `__config`. The fact that we duplicated the check in `src/chrono.cpp` and `src/filesystem/operations.cpp` is bad.



================
Comment at: libcxx/src/chrono.cpp:13
 #include <time.h>        // clock_gettime, CLOCK_MONOTONIC and CLOCK_REALTIME
+#include <unistd.h>
 #include "include/apple_availability.h"
----------------
`<unistd.h>` is not supported on Windows AFAICT, so you'll need to guard this with an appropriate `#if`.


================
Comment at: libcxx/src/chrono.cpp:16
 
-#if !defined(__APPLE__)
+#if _POSIX_TIMERS > 0
 #define _LIBCPP_USE_CLOCK_GETTIME
----------------
It turns out the `__APPLE__` check is still required. See `libcxx/src/include/apple_availability.h:33` & friends. Thus, I would use:

```
#if !defined(__APPLE__) && defined(_POSIX_TIMERS)
```


================
Comment at: libcxx/src/chrono.cpp:18
 #define _LIBCPP_USE_CLOCK_GETTIME
-#endif // __APPLE__
+#endif // _POSIX_TIMERS
 
----------------
Please remove the comment at the closing `#endif`. For one-liner `#if`s like that, it's just an occasion to get the actual condition and the comment out of sync. I know you were just being consistent with surrounding code, but it's bad that we've been doing it.


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

https://reviews.llvm.org/D79305