bogner added a comment. In D157151#4577790 <https://reviews.llvm.org/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 place - this is a much needed and long overdue > refactor. > > There's quite a bit of churn here, so I will need a few days to scan. In the > meantime, could you update flang/docs/FlangDriver.md? And in general, please > note that this updates (primarily) `clangDriver` logic, which is used by both > by Clang and Flang. In particular, Options.td is shared. I think that it's > worth highlighting that this change benefits both sub-projects. Yep, sorry about that. I tried to break this up as much as I could but given the number of places that depend on Options.td it was tricky. I do think this should makes things much easier for flang's driver in particular, as well as making clang-cl and clang-dxc easier to deal with. Sorry I missed updating FlangDriver.md - FWIW I did build flang and run those tests with this change. I'll update the patch with doc updates momentarily. >> introduced in llvm Option > > Could you add a link? I've updated the description to mention https://reviews.llvm.org/D157149. ================ Comment at: clang/include/clang/Driver/Options.h:25-37 - CoreOption = (1 << 8), - CLOption = (1 << 9), - CC1Option = (1 << 10), - CC1AsOption = (1 << 11), - NoDriverOption = (1 << 12), - LinkOption = (1 << 13), - FlangOption = (1 << 14), ---------------- awarzynski wrote: > What happens to `CoreOption`s? Same for `NoDriverOption`s? - `CoreOption` meant "available in clang and clang-cl (and maybe dxc...)", so it's now `[Default, CLOption]`. - `NoDriverOption` is `!Default` - negative flags were a big part of why the include/exclude thing was awkward, so the change in https://reviews.llvm.org/D157149 introduced a bit for options where you don't say anything specific, which meant the clang driver in practice. For a complete map, see `flag_to_vis` in the `update_options_td_flags.py` helper script I provided for downstreams: https://reviews.llvm.org/D157151#change-FMUYy4yRDQLQ ================ Comment at: clang/include/clang/Driver/Options.td:193 def m_x86_Features_Group : OptionGroup<"<x86 features group>">, - Group<m_Group>, Flags<[CoreOption]>, DocName<"X86">; + Group<m_Group>, Vis<[Default, CLOption, DXCOption]>, + DocName<"X86">; ---------------- awarzynski wrote: > What's `Default` in `Vis<[Default, CLOption, DXCOption]>,`? If you don't specify any visibility, the visibility is "Default" (See https://reviews.llvm.org/D157149). For all intents and purposes this means "Clang Driver" in this file. 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
