[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