[PATCH] D55296: [Support] Fix GNU/kFreeBSD build

Kamil Rytarowski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 4 14:21:21 PST 2018


krytarowski added inline comments.


================
Comment at: lib/Support/Unix/Path.inc:59
 #if !defined(__APPLE__) && !defined(__OpenBSD__) && !defined(__FreeBSD__) &&   \
-    !defined(__linux__)
+    !defined(__linux__) && !defined(__FreeBSD_kernel__)
 #include <sys/statvfs.h>
----------------
jrtc27 wrote:
> krytarowski wrote:
> > jrtc27 wrote:
> > > krytarowski wrote:
> > > > efriedma wrote:
> > > > > Does this make the `!defined(__FreeBSD__)` check redundant?
> > > > Technically yes.
> > > Unfortunately not. Plain FreeBSD just defines `__FreeBSD__`, and because that indicates both kernel and userland, `__FreeBSD_kernel__` was introduced just for GNU/kFreeBSD. In theory it should also be defined for pure FreeBSD, but it isn't. Maybe someone will propose that and we can drop this in 10 years' time, but today is not that day.
> > FreeBSD defines it in `sys/param.h`.
> > 
> > https://github.com/freebsd/freebsd/blob/467e55b8abbf80cd144961ee51833b8ff4209b17/sys/sys/param.h#L65
> Huh, TIL. I wonder why this isn't put in the compiler(s) though... Anyway, like it says, we should still really be checking for `__FreeBSD__`.
GNU/kFreeBSD should be detected imho with `defined(__FreeBSD_kernel__) && defined(__GLIBC__)`.. but since GNU/kFreeBSD is an ongoing experiment I recommend to keep using standalone check for `__FreeBSD__` and another for `__FreeBSD_kernel__` with assumption that this is GNU/kFreeBSD.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55296





More information about the llvm-commits mailing list