dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed.
When are you planning to refactor `OptInFFlag` to support the use cases called out in this patch? If it's right away / soon, I think it'd be a bit cleaner for to land the refactoring first, then the options that use it, as opposed to fixing them up later. WDYT? (If you're not planning to do it until "later", I don't feel strongly, but as noted inline I'm not sure we need all the todos.) ================ Comment at: clang/include/clang/Driver/Options.td:2987 def pthreads : Flag<["-"], "pthreads">; +// todo: simplify these into a version of OptInFFlag that doesn't imply `f` name prefix def pthread : Flag<["-"], "pthread">, Flags<[CC1Option]>, ---------------- I wonder if this `TODO` is helpful, or if you could find these later just by looking for `def no_` thanks to the consistent naming that's used? ================ Comment at: clang/include/clang/Driver/Options.td:3688 // C++ SYCL options +// todo: simplify these with a variant of OptInFFlag that doesn't add Group def fsycl : Flag<["-"], "fsycl">, Group<sycl_Group>, Flags<[CC1Option, CoreOption]>, ---------------- Similarly I think a grep for `def.*no_` would turn this one up (probably the rest too). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83979/new/ https://reviews.llvm.org/D83979 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
