[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.

> 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).

Alternatively just let people shoot themselves in the foot, a documentation change regarding the dangers of the flag should suffice. 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 (which is a very specific and narrow edge case to allow people to collect performance data). Outside of that very specific case, have we really had many issues with consumers accidentally setting weird flags that they would have to discover in the first place. I don't have a strong opinion on `-Xclang` but `-mllvm <option>` is verbose enough to use as is. Maybe it should be made a lot more obvious in one way or another but a warning by default seems like it's taking rather drastic preemptive measures against a non-issue (do correct me if I'm wrong here).

I do agree that the documentation should definitely scream that it's not stable and it's intended for maintainers since it's a convinient interface (the `-mllvm` one) for passing flags through the driver all the way to things like the MC layer, where needed, and yes these flags can be removed without notice, but again, I don't think it's our responsibility to protect users from using *intentionally* hidden flags aside from documenting the reason for why they're intentionally hidden in a more obvious way, I think the fact that they are intentionally not shown in standard LLVM help and yet are available contradicts the idea behind this patch, the whole purpose of the flag is to directly interact with specific internal parts of LLVM which in itself is not really intended to be a stable interface.


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

https://reviews.llvm.org/D55150