Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
| lib/Support/Unix/Path.inc | ||
|---|---|---|
| 59 ↗ | (On Diff #176705) | Does this make the !defined(__FreeBSD__) check redundant? |
| lib/Support/Unix/Path.inc | ||
|---|---|---|
| 59 ↗ | (On Diff #176705) | Technically yes. |
| lib/Support/Unix/Path.inc | ||
|---|---|---|
| 59 ↗ | (On Diff #176705) | 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. |
| lib/Support/Unix/Path.inc | ||
|---|---|---|
| 59 ↗ | (On Diff #176705) | FreeBSD defines it in sys/param.h. https://github.com/freebsd/freebsd/blob/467e55b8abbf80cd144961ee51833b8ff4209b17/sys/sys/param.h#L65 |
| lib/Support/Unix/Path.inc | ||
|---|---|---|
| 59 ↗ | (On Diff #176705) | 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__. |
| lib/Support/Unix/Path.inc | ||
|---|---|---|
| 59 ↗ | (On Diff #176705) | 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. |
| lib/Support/Unix/Path.inc | ||
|---|---|---|
| 59 ↗ | (On Diff #176705) | Well, this looks like a kernel-based check, not a userland check, as it's pulling in stuff from sys/? Everything here is checking kernel macros (__CYGWIN__ is an odd-ball that's sort of both), including __GNU__ which by your suggestion should be __GNU__ && __GLIBC__; ditto for __linux__. So whilst I would agree with your suggestion on a technical level, I don't think it actually applies here? |
| lib/Support/Unix/Path.inc | ||
|---|---|---|
| 59 ↗ | (On Diff #176705) | kFreeBSD could be in theory used with a different userland than GNU or BSD, but it's a question whether it would solve any new problems. __GNU__ && __GLIBC__ would be more purist and there is __gnu_hurd__ for this exact purpose, similar to __gnu_linux__. Actually checking for __FreeBSD_kernel__ && __GLIBC__ is even recommended by https://sourceforge.net/p/predef/wiki/OperatingSystems/ It's a similar debate whether Debian is Linux or GNU/Linux. For practical purposes the proposed patch is the right thing to do. |
| lib/Support/Unix/Path.inc | ||
|---|---|---|
| 59 ↗ | (On Diff #176705) | Yes, but that's only a valid thing to be checking if you care about the userland; my point is that this is very much a kernel check (or at least, that's what it's checking on every other platform here; e.g. __linux__ is there on its own without a __GLIBC__ check) and so we explicitly *don't* want to be checking whether it's GNU/kFreeBSD, musl/kFreeBSD or something stupid like Android/kFreeBSD. |
| lib/Support/Unix/Path.inc | ||
|---|---|---|
| 59 ↗ | (On Diff #176705) | This is not userland check but OS check. GNU/kFreeBSD composes an OS (kernel + userland). NetBSD/FreeBSD/Linux/Apple copose a different OS. Mixing userlands and kernels does not buy much in real world so kFreeBSD is the sole experiment out there (ignoring now some alternative libc userlands for small embedded Linux world). Android check is a check for Android kernel and Android userland. |