awarzynski added a comment. Thank you all for you comments! Please find my replies below. I've picked 4 main points raised here.
1 - In D89799#2342677 <https://reviews.llvm.org/D89799#2342677>, @rnk wrote: > This seems like pretty corner case functionality. Do we really need this > diagnostic? Good point. If nobody objects I will remove it. 2 - In D89799#2344672 <https://reviews.llvm.org/D89799#2344672>, @yaxunl wrote: > I am not sure whether it is proper to rename it. > > Originally, this flag means driver option which is not supposed to be > forwarded to tools. It is more like a reminder to driver developers since > clang driver does not automatically forward options to tools and does not > enforce not forwarding options with this flag to tools. I'm happy to rename this differently, e.g. `DontForwardToTools`. However, `DriverOption` is very confusing to me. Which driver is this flag referring to? Compiler driver (`clang`) ? Frontend driver (`clang -cc1`)? If it refers to libclangDriver in general, then everything in `Options.td` is a driver flag by default (i.e. it belongs to the library the implements the compiler driver API). Also, based on the current usage and feedback from [1]: > It served two purposes (1) (@awarzynski: No longer applies) (2) whether an > option should be forwarded for -Xarch -Xarch_host -Xarch_device (macOS and > CUDA use cases) I feel that `DriverOption` is currently too generic. 3 - In D89799#2344672 <https://reviews.llvm.org/D89799#2344672>, @yaxunl wrote: > Then some toolchains use this flag as a heuristic not to use options with > this flag with -Xarch. For that purpose it is too broad as many options with > this flag can be used with -Xarch. Isn't there a more broader problem with various flags being used incorrectly in Options.td? I would love to help improving that, but I'm new to this codebase. Are there any heuristics that I could use to guide me in that? For example - how do I decide where to delete `DriverOption` from? Btw, are you suggesting that this change will be incompatible with some downstream projects? 4 - In D89799#2344672 <https://reviews.llvm.org/D89799#2344672>, @yaxunl wrote: > So the question is: Is there any value to keep the original intention of this > flag, i.e. mark some options as driver options without enforcing it? Or do we > want to add assertions or warnings in clang -cc1 to check if driver options > are passed to FE? Isn't this already enforced in `ToolChain::TranslateXarchArgs` (via the diagnostic)? That's the only place where `DriverOption` is currently used. Also, FE? Thank you for reviewing! [1] http://lists.llvm.org/pipermail/cfe-dev/2020-October/067023.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89799/new/ https://reviews.llvm.org/D89799 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
