[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job
Joseph Huber via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 27 15:12:35 PST 2023
jhuber6 added a comment.
Thanks for the changes.
================
Comment at: clang/lib/Driver/Driver.cpp:4226
+ Args.ClaimAllArgs(options::OPT_cl_ignored_Group);
+ }
----------------
nit. remember to `clang-format`
================
Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:170
+
+Tool *clang::driver::toolchains::HLSLToolChain::getTool(
+ Action::ActionClass AC) const {
----------------
I feel like this logic should go with the other random `Tool` versions we have in `ToolChain.cpp`. See `ToolChain.cpp:440` and there should be examples of similar tools.
================
Comment at: clang/lib/Driver/ToolChains/HLSL.h:53
static std::optional<std::string> parseTargetProfile(StringRef TargetProfile);
+ bool needValidation(llvm::opt::DerivedArgList &Args) const;
+
----------------
Is `needValidation` a good name here? It's asking more like `hasValidator` or `supportsValidation`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141705/new/
https://reviews.llvm.org/D141705
More information about the cfe-commits
mailing list