rnk added inline comments.
================
Comment at: clang/lib/Driver/Distro.cpp:205
+ const llvm::Triple &TargetOrHost) {
+ static Distro::DistroType Type = Distro::UninitializedDistro;
+
----------------
rnk wrote:
> aganea wrote:
> > I'm not sure if this is safe. @rnk Do we expect to call `GetDistro` with a
> > different `VFS` or a different `Triple` when compiling with Clang? For GPU
> > targets? Shouldn't we do this higher up, on the call site?
> I think there is a plausible use case for a compile server that does two
> compiles with different VFSs where you would get different distro detection
> results. I think this should be a field on the `Driver` object instead of a
> global variable.
>
> I know LLD uses lots of globals, but that's not part of the clang design
> philosophy.
Sorry, I spoke too soon.
For @aganea's concern, my understanding is that the distro is a property of the
host machine doing the compilation. Even if we do two compiles in the same
process on the same machine with two triples, they should have the same distro,
if they are using the same real FS. At least, the FS shouldn't change out from
under us in a way that changes the result of distro detection. :) I think it's
reasonable to use distro detection to inform header search paths, but we should
avoid using it to control properties of the output, like stack protection
security features, for example. Otherwise, it becomes impossible to target one
Linux distro from another, because the distro does not appear to be
controllable by a command line flag.
And, now that I see what is happening with the real FS checks here, I withdraw
my concern. Even if you did make this a property of the driver, I would want
some global variable somewhere to check that we don't end up doing the distro
check twice. It's the only way to really be sure that some passerby doesn't
introduce a new distro check. =P
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87187/new/
https://reviews.llvm.org/D87187
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
