[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

In D55150#1321734 <https://reviews.llvm.org/D55150#1321734>, @thakis wrote:

> I don't have an opinion on this patch (if you force me to have one, I'm weakly in favor), but I agree with the general sentiment. When I told people to not use mllvm and Xclang before, they told me "but if I'm not supposed to use them, why are they advertised in --help"?
>
>   thakis at thakis:~/src/llvm-build$ bin/clang --help | grep Xclang
>     -Xclang <arg>           Pass <arg> to the clang compiler
>   thakis at thakis:~/src/llvm-build$ bin/clang --help | grep mllvm
>     -mllvm <value>          Additional arguments to forward to LLVM's option processing
>
>
> Which to me is a valid point! Maybe we should remove them from --help or say "internal only, don't usually use" there?


I think the documentation for `-mllvm` could definitely emphasize the aspect that these flags are not a part of any public or stable interface and are used to change internal behavior of LLVM components a lot more. Definitely in favor of doing that.

In D55150#1321724 <https://reviews.llvm.org/D55150#1321724>, @jyknight wrote:

> In D55150#1321705 <https://reviews.llvm.org/D55150#1321705>, @kristina wrote:
>
> > > There is a cost to having people encode these flags into their build systems -- it can then cause issues if we ever change these internal flags. I do not think any Clang maintainer intends to support these as stable APIs, unlike most of the driver command-line. But, neither -Xclang nor -mllvm obviously scream out "don't use this!", and so people may then add them to their buildsystems without causing reviewers to say "Wait...really? Are you sure that's a good idea?".
> >
> > This is why I proposed a compromise, allow this warning to be disabled completely for people actively using those flags, which are pretty much exclusively toolchain developers (well basically what I proposed, since it's not clear what counts as a build made by someone who is working and debugging a pass, being fully aware of those flags, using the subset of them specific to that pass/feature, I would say assertion+dump enabled builds are closest, but having an explicit build flag for it would be nicer). It's more unified than everyone either adding workarounds into build systems or disabling it completely (by just removing it).
>
>
> I mean, I'm not much opposed to adding that -- just that any new build-time options is always a minor shame. But you didn't respond to the other part of my message -- is adding `-Wno-experimental-driver-option` to your compile-flags not already a completely sufficient solution for your use-case?
>
> > Besides, I don't think this really ever surfaced as an issue, until Chandler's idea with regards to an unsupressable warning for performance tests for 7.x.x
>
> The primary impetus for me was actually the discovery that Boost's build system was using "-Xclang -include-pth" a few weeks back.


It would be an inconvenience to developers and a lot of patches grow from downstream, given that some people may want to disable the flag for development, it would basically mean that some may end up with varying workarounds as opposed to a uniform one. I don't think that happening would be of benefit to anyone. And yes I could add an extra flag for that configuration in the build system, in my case, but I don't think I'll be the only one doing that so again, this would just cause more fragmentation. I don't see that as a good thing.


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

https://reviews.llvm.org/D55150