[PATCH] D34249: [libc++] Don't use UTIME_OMIT to detect utimensat on Apple

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.


================
Comment at: src/experimental/filesystem/operations.cpp:23-28
+// We can use the presence of UTIME_OMIT to detect platforms that do not
+// provide utimensat, with some exceptions on OS X.
+#if !defined(UTIME_OMIT) || \
+   (defined(__MAC_OS_X_VERSION_MIN_REQUIRED) && __MAC_OS_X_VERSION_MIN_REQUIRED < 1030)
+#define _LIBCPP_HAS_NO_UTIMENSAT
+#endif
----------------
dexonsmith wrote:
> EricWF wrote:
> > dexonsmith wrote:
> > > Sadly this isn't quite sufficient.  As per Jack's suggested SDK patch in the PR, we need to enumerate the platforms :/.  I think this should be the right logic for the four Darwin platforms:
> > > 
> > >     (defined(__IPHONE_OS_VERSION_MIN_REQUIRED) && __IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_11_0) || \
> > >     (defined(__WATCH_OS_VERSION_MIN_REQUIRED) &&  __WATCH_OS_VERSION_MIN_REQUIRED < __WATCHOS_4_0)  || \
> > >     (defined(__TV_OS_VERSION_MIN_REQUIRED) && __TV_OS_VERSION_MIN_REQUIRED < __TVOS_11_0)           || \
> > >     (defined(__MAC_OS_X_VERSION_MIN_REQUIRED) && __MAC_OS_X_VERSION_MIN_REQUIRED < __MAC_10_13)
> > > 
> > Do we have to do the below dance for all of those macros?
> > 
> > ```
> > #if !defined(__FOO_VERSION_MIN_REQUIRED) && defined(__ENVIROMENT_FOO_VERSION_MIN_REQUIRED)
> > #define __FOO_VERSION_MIN_REQUIRED __ENVIROMENT_FOO_VERSION_REQUIRED
> > #endif
> > ```
> Nope.  I just advised you to use the wrong ones.  Use the `__ENVIRONMENT` versions pre-defined by Clang.
Specifically:

```
clang/lib/Basic/Targets.cpp:183:      Builder.defineMacro("__ENVIRONMENT_TV_OS_VERSION_MIN_REQUIRED__", Str);
clang/lib/Basic/Targets.cpp:185:      Builder.defineMacro("__ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__",
clang/lib/Basic/Targets.cpp:197:    Builder.defineMacro("__ENVIRONMENT_WATCH_OS_VERSION_MIN_REQUIRED__", Str);
clang/lib/Basic/Targets.cpp:221:    Builder.defineMacro("__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__", Str);
```


https://reviews.llvm.org/D34249