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

Justin Bogner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 14 03:38:08 PDT 2023


bogner added a comment.

In D157151#4583904 <https://reviews.llvm.org/D157151#4583904>, @awarzynski wrote:

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

Ah right. Yes, this is exactly the problem that this patch is trying to fix. Glad to see that it seems to be working as intended for flang-new!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157151



More information about the llvm-commits mailing list