[PATCH] D54883: [Support/FileSystem] Use st_atimespec only when __APPLE__ is defined

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D54883#1307857 <https://reviews.llvm.org/D54883#1307857>, @aaron.ballman wrote:

> In D54883#1307853 <https://reviews.llvm.org/D54883#1307853>, @krytarowski wrote:
>
> > In D54883#1307787 <https://reviews.llvm.org/D54883#1307787>, @aaron.ballman wrote:
> >
> > > Do our minimum system requirements for building LLVM include requiring POSIX 2008 conformance (perhaps indirectly, through the minimum toolchain requirements)?
> >
> >
> > There are almost no OSes having conformance with UNIX/POSIX. UNIXv6 (or similar) is Solaris only.
>
>
> If our existing system requirements aren't sufficient to practically guarantee these fields are available, I'd prefer to keep the autodetect code and rely on that instead.


There are 2 pieces of autodetect code:

1. preprocessor level detection `#if defined(HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC)`
2. cmake detection `CHECK_STRUCT_HAS_MEMBER`

After this patch, they get simplified and become:

1. preprocessor level detection `#ifdef __APPLE__`

On https://github.com/apple/darwin-xnu/blob/master/bsd/sys/stat.h , I cannot find a better feature macro to test so I have to use `__APPLE__`. When they start to support `st_atim` (which is widely available on other platforms) in a newer version, we can make the `#ifdef __APPLE__` check more specific.

The patch directly tests for the necessary prerequisite and removes two cmake variables. This makes it substantially more build system agnostic (for people who use gn,bazel,... they don't need to re-invent the feature detection mechanism in their build systems) and probably (ironically) more portable.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54883