[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

nikic added a comment. To avoid a full revert, I removed the deprecations in this patch in https://github.com/llvm/llvm-project/commit/348d36f1a2c8c776ce87e038bf15fc4cabd62702. Please do not deprecate methods that have remaining in-tree users and make sure that `-Werror=deprecated-declarations`

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

bogner added a comment. In D157151#4587522 , @awarzynski wrote: > This might cause some disruption to downstream consumers of this API and > Options.td. Hopefully, "update_options_td_flags.py" that you've included > should minimise that. I suggest "adv

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

awarzynski accepted this revision. awarzynski added a comment. This revision is now accepted and ready to land. While quite extensive, I find the overall logic in this patch very well structured and executed in a very clean manner. It removes a lot of ambiguity, makes the overall design much eas

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

bogner added a comment. In D157151#4583904 , @awarzynski wrote: > This can be tweaked in `getOptionVisibilityMask` (extracted from this patch): > > llvm::opt::Visibility > Driver::getOptionVisibilityMask(bool UseDriverMode) const { > if (!UseDriv

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

awarzynski added a comment. In D157151#4582441 , @bogner wrote: > This is a little bit complicated by the fact that the Option library is > already partially extracted out of clang (and used for other drivers, like > lld and lldb). The "Default" visibil

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

bogner added a comment. In D157151#4582109 , @awarzynski wrote: > I think that it would be good to replace `Default` with e.g. > > - `Clang`, or > - `ClangDriver`, or > - `ClangCompilerDriver`. > > Or, at least, to make the meaning of `Default` much clea

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

awarzynski added a comment. I've tested this locally and can confirm that the behavior of `clang` and `flang-new` has been preserved (as in, these changes won't be visible to the end users). Nice! I think that it would be good to replace `Default` with e.g. - `Clang`, or - `ClangDriver`, or -

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

bogner added a comment. In D157151#4577790 , @awarzynski wrote: > Hey @bogner , I've only skimmed through so far and it's looking great! That > Include/Exclude API was not fun to use. What you are proposing here takes > Options.td to a much a better pl

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

awarzynski added a comment. Hey @bogner , I've only skimmed through so far and it's looking great! That Include/Exclude API was not fun to use. What you are proposing here takes Options.td to a much a better place - this is a much needed and long overdue refactor. There's quite a bit of churn