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

efriedma added a comment. > In fact -mllvm is used extensively in a lot of lit/FileCheck tests, so that's > also going to cause problems. I count 13 uses of -mllvm in driver commands in-tree, and some of those are actually testing the flag itself. That doesn't seem like a lot. (This is exclu

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

NoQ added a comment. In D55150#1321793 , @jyknight wrote: > In D55150#1321759 , > @george.karpenkov wrote: > > > Using `-Xclang` is the only way to pass options to the static analyzer, I > > don't think we should

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

jyknight added a comment. In D55150#1321829 , @efriedma wrote: > I'm not sure that putting a warning that can be disabled really helps here; > anyone who needs the option will just disable the warning anyway, and then > users adding additional options so

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

kristina requested changes to this revision. kristina added a comment. In D55150#1321829 , @efriedma wrote: > I'm not sure that putting a warning that can be disabled really helps here; > anyone who needs the option will just disable the warning anyway, a

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

efriedma added a comment. Or actually, if you really want to discourage people from using them, maybe we could use the LLVM version number in the name, like "--unstable-llvm-option-8" (which would change to "--unstable-llvm-option-9" in in 9.0, etc.). This would allow developers to continue us

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

george.karpenkov added a comment. > Well,, that seems unfortunate if we have the only supported interface for the > static analyzer be an internal interface. Perhaps it can be given a different > option? Even discounting this change, I that seems like it would be > appropriate. Officially ther

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

efriedma added a comment. I'm not sure that putting a warning that can be disabled really helps here; anyone who needs the option will just disable the warning anyway, and then users adding additional options somewhere else in the build system will miss the warning. Instead, it would probably

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

jyknight added a comment. In D55150#1321759 , @george.karpenkov wrote: > Using `-Xclang` is the only way to pass options to the static analyzer, I > don't think we should warn on it. Well,, that seems unfortunate if we have the only supported interface

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

kristina added a comment. In 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, th

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

arphaman requested changes to this revision. arphaman added a comment. Swift uses `-Xclang` to pass in build settings to its own build and to pass in custom options through its Clang importer that we intentionally don't want to expose to Clang's users. We don't want to warn for those uses for su

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

george.karpenkov added reviewers: dcoughlin, NoQ. george.karpenkov requested changes to this revision. george.karpenkov added a comment. Using `-Xclang` is the only way to pass options to the static analyzer, I don't think we should warn on it. CHANGES SINCE LAST ACTION https://reviews.llvm.o

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

erichkeane added a comment. I'm in favor of at least the documentation changes, though I'd like to see them use stronger language. I'm fairly in favor of the warning itself since its easy enough to disable (with the -Wno flag), so I don't terribly understand those against it. CHANGES SINCE L

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

thakis added a comment. Oh hey, the patch does that already! Ignore me, then :-) That part is probably less controversial, maybe you want to land that while the warning part is being discussed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55150/new/ https://reviews.llvm.org/D55150

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

arsenm added a comment. I don't really see the point of this and think it will just be an inconvenience to llvm developers. Another use case we have for using these in a build system is for the builtin library shipped with the compiler CHANGES SINCE LAST ACTION https://reviews.llvm.org/D551

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

thakis added a comment. 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"?

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

aaron.ballman added inline comments. Comment at: clang/docs/UsersManual.rst:3137 -W Enable the specified warning - -XclangPass to the clang compiler + -XclangPass to the clang cc1 frontend. For experimental use only. -

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

jyknight added a comment. In 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 maintaine

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

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.

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

jyknight added a comment. In D55150#1321046 , @kristina wrote: > Personally I'm against this type of warning as it's likely anyone using > `-mllvm` is actually intending to adjust certain behaviors across one or more > passes with a lot of switches suppo

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

kristina added a comment. Personally I'm against this type of warning as it's likely anyone using `-mllvm` is actually intending to adjust certain behaviors across one or more passes with a lot of switches supported by it being intentionally hidden from ` --help` output requiring explicitly spe

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

jyknight added a comment. In D55150#1318082 , @chandlerc wrote: > I think this should be `internal-driver-option` and the text updated? I don't > think these are necessarily experimental, they're internals of the compiler > implementation, and not a supp

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

chandlerc requested changes to this revision. chandlerc added a comment. This revision now requires changes to proceed. I think this should be `internal-driver-option` and the text updated? I don't think these are necessarily experimental, they're internals of the compiler implementation, and no

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

jyknight created this revision. jyknight added reviewers: rsmith, chandlerc. Herald added a subscriber: arphaman. This warning, Wexperimental-driver-option, is on by default, exempt from -Werror, and may be disabled. Additionally, change the documentation to note that these options are not intend