[PATCH] D77776: [Driver] Default to libc++ on FreeBSD
Dimitry Andric via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 31 05:43:48 PDT 2020
dim added a comment.
In D77776#1993211 <https://reviews.llvm.org/D77776#1993211>, @arichardson wrote:
> I don't like the fact that this only changes one of the users of `getTriple().getOSMajorVersion()`.
Why not, if this is a one-off case, it's perfectly OK to put it in here. Maybe add a comment as to why it is needed?
Could you add a new member function such as
> void FreeBSD::getMajorVersion() const {
> unsigned Major = getTriple().getOSMajorVersion();
> if (Major == 0)
> return 10;
> return Major
> }
>
> and replace all uses of `getTriple().getOSMajorVersion()` with `getMajorVersion()`.
Maybe other OSes would also have this same issue, but then you'd have to replace this kind of logic for *all* of them, not only FreeBSD. That said, maybe there aren't so many places where the major version is checked, and where features are enabled or disabled depending on this version?
> We could also use the host version instead of 10?
>
> +#ifdef __FreeBSD__
> + return __FreeBSD_version / 100000;
> +#else
> + return 10;
> +#endif
No, that would hardcode something at build time. We need to respect whatever triple the user is passing in.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77776/new/
https://reviews.llvm.org/D77776
More information about the cfe-commits
mailing list