[PATCH] D141705: [HLSL] [Dirver] add dxv as a Driver Action Job

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:4217-4218
+    // Only add action when needValidation.
+    const auto &TC = getToolChain(Args, C.getDefaultToolChain().getTriple());
+    const auto *HLSLTC = static_cast<const toolchains::HLSLToolChain *>(&TC);
+    if (HLSLTC->needValidation(Args, *this)) {
----------------
This can create a new `ToolChain`. We probably don't want that. Maybe someone more familiar with this compilation mode can chime in. But my guess is that when we created the `Compilation`.

This code will only fire if the above triple matches for the default ToolChain. So we should be safe in just casting it directly.


================
Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:258
+bool HLSLToolChain::needValidation(DerivedArgList &Args,
+                                   const Driver &D) const {
+  if (Args.getLastArg(options::OPT_dxc_disable_validation))
----------------
We don't need to pass in `D` since this is no longer static. You can use `getDriver()`


================
Comment at: clang/test/Driver/dxc_dxv_path.hlsl:19
+
+// PHASES:+- 0: input, "[[INPUT:.+]]", hlsl
+// PHASES-NEXT:+- 1: preprocessor, {0}, c++-cpp-output
----------------
nit. remove the `+-` and add whitespace to make it more readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141705