[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
----------------
ro wrote:
> 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.
I've now filed Issue #53919 so the information is in one place rather than spread over several reviews.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119695