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

Justin Bogner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Aug 12 11:46:28 PDT 2023


bogner added a comment.

In D157151#4582109 <https://reviews.llvm.org/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 clearer (through e.g. comments). In general, I feel that `Default` is skewed towards this notion that `clang` (the compiler driver tool) is the main client of `clangDriver`. That used to be the case, but with Flang's driver also implemented in terms of `clangDriver` , that's no longer the case. Also, the long term goal is to extract the driver library out of Clang. One day :)

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" visibility is defined in llvm's Option library, so calling it something like "Clang" would be pretty confusing for users that aren't the clang Driver library. I suppose one option would be to throw something like `using ClangDriver = llvm::Driver::Default;` in Options.h inside the `clang::driver::options` namespace, and then using the alias in Options.td. Would that help?

> Also, note that `Default` options will be available in both `clang` and `flang-new`. That's the case today (so not something affected by your changes). Ideally, Flang should be able to disable those _Clang options_. That's not possible ATM. Contrary to Flang, Clang can disable _Flang options_ with `FlangOnlyOption`. There is no such flag for Flang ATM, but I think that we could re-use `Default` for that. WDYT?

`Default` options are either options that don't specify `Vis` at all or explicitly mention `Default`. They are not visible in `flang-new` after this change unless they also specify `FlangOption`. Specifically, instead of having to deal with `FlangOnlyOption` we explicitly opt in to default. Some examples:

  def default : Flag<["-"], "a">; //visible only in invocations of `clang`
  def default_explicit : Flag<["-"], "b">, Vis<[Default]>; //visible only in invocations of `clang`
  def flang_only : Flag<["-"], "c">, Vis<[FlangOption]>; // only `flang-new`
  def flang_and_clang : Flag<["-"], "d">, Vis<[Default, FlangOption]>; // Both `clang` and `flang-new`
  def flang_fc1 : Flag<["-"], "e">, Vis<[FC1Option]>; // `flang-new -fc1`, but not the `flang-new` driver or `clang`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157151



More information about the cfe-commits mailing list