awarzynski added a comment.
In D157151#4582441 <https://reviews.llvm.org/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" 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.
Ah, I see. I guess `clangDriver` is a bit unique in the sense that there's
quite a few drivers that rely on its Options.td:
- `clang,` `clang -cc1`, `clang -cc1as`, `clang-cl`,
- `flang-new`, `flang-new -fc1`.
> 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?
That would make sense to me, yes. Otherwise the meaning of `Default` is
unclear. These things tend to be obvious to folks familiar with the
implementation. However, Options.td would normally be edited/updated by people
less familiar with the fine details and more concerned about their favorite
option and how it's implemented in their favorite tool.
> `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`.
That's not quite true ATM. Although not used, the following C++ option _is
visible_ in `flang-new`:
flang-new -fno-experimental-relative-c++-abi-vtables file.f90
flang-new: warning: argument unused during compilation:
'-fno-experimental-relative-c++-abi-vtables'
This can be tweaked in `getOptionVisibilityMask` (extracted from this patch):
llvm::opt::Visibility
Driver::getOptionVisibilityMask(bool UseDriverMode) const {
if (!UseDriverMode)
return llvm::opt::Visibility(llvm::opt::Default);
if (IsCLMode())
return llvm::opt::Visibility(options::CLOption);
if (IsDXCMode())
return llvm::opt::Visibility(options::DXCOption);
if (IsFlangMode()) {
// vvvvvvvvvvvvvvv HERE vvvvvvvvvvvvvvvvvvv
// TODO: Does flang really want *all* of the clang driver options?
// We probably need to annotate more specifically.
return llvm::opt::Visibility(llvm::opt::Default | options::FlangOption);
}
return llvm::opt::Visibility(llvm::opt::Default);
}
Now, prior to this change I couldn't find a way to disable unsupported Clang
options in Flang. With this patch,
- all Clang options gain a visibility flag (atm that's `Default`),
- that flag can be used to disable options unsupported in Flang.
For this to work, all options supported by Flang need to have their visibility
flags set accordingly. That's the goal, but I can see that we have missed quite
a few options over the time (*). Updated here: https://reviews.llvm.org/D157837.
(*) There was no mechanism to enforce that. This is now changing.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157151/new/
https://reviews.llvm.org/D157151
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
