[PATCH] D119695: [Support] Fix build on illumos

[Rainer Orth via Phabricator via llvm-commits]

ro added inline comments.


================
Comment at: llvm/lib/Support/Unix/Path.inc:121
 
+#if defined(__sun__)
+// The madvise() declaration on Illumos cannot be made visible if _XOPEN_SOURCE
----------------
jclulow wrote:
> ro wrote:
> > The canonical way to check for Solaris/Illumos is `defined(__sun__ && defined(__svr4__)`.
> > 
> > As issues like this don't come up for the first time, it would be extremely helpful if Illumos would provide a predefine to support that, something like `__illumos__` or whatever the community sees fit.
> We are part way through making that happen with [13726 distinguish ourselves
> with a macro](https://www.illumos.org/issues/13726).  Indeed, the definition is already exposed by GCC 10.X, the default compiler shipped with OmniOS r151038 LTS systems (a popular distribution):
> 
> ```
> $ head -1 /etc/release
>   OmniOS v11 r151038ag
> $ gcc --version | head -1
> gcc (OmniOS 151038/10.3.0-il-1) 10.3.0
> $ gcc -E - <<< 'is this __illumos__?' | tail -1
> is this 1?
> ```
> 
> We do still need to circle back around and get a fallback definition added to a core header file -- but we're definitely committed to `__illumos__` and you should be able to start using it.
> 
That's excellent news, thanks. It will certainly simplify things in the future, especially in codebases like LLVM that often avoid configure/cmake tests in favor of hardcoding info based on target macros.

I think there are two more areas that need to be dealt with:
- `clang` needs to distinguish at runtime if it's compiling for Solaris or Illumos.  Right now, we have `isOSSolaris()` which just looks at the os element of the triple (which is still identical between the two).
- In many `cmake` files we have checks like `${CMAKE_SYSTEM_NAME} MATCHES "SunOS"`. In some cases, the two will need to be distinguished there, too.

I guess the general model to follow will be what was done for Go: the build tag `solaris` is true for both, while a separate one (`illumos`) can be used to disambiguate if need be.

I have an idea how to go about with the introduction of `isOSIllumos()`, but that needs to be flashed out with the LLVM maintainers once there's general agreement on the direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119695