[PATCH] D123884: [HLSL][clang][Driver] Support target profile command line option.

Chris Bieneman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 26 15:09:37 PDT 2022


beanz added a comment.

I pointed out a few (not all) the places where you have unneeded brackets. Also all your new files don't have newlines at the end of them (the C standard specifies that as a requirement although pretty much all compilers just issue a warning).



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:843
+  // HLSL related end of code gen work items.
+  if (LangOpts.HLSL) {
+    getHLSLRuntime().finishCodeGen();
----------------
MaskRay wrote:
> https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
As @MaskRay said you don't need the bracket here.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3469
+      A->renderAsInput(Args, CmdArgs);
+    }
+}
----------------
Omitting the bracket around the for makes this even more confusing... please omit the brackets around the if too.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6220
+    RenderHLSLOptions(Args, CmdArgs, InputType);
+  }
+
----------------
no brackets here either


================
Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:171
+        continue;
+      }
+    }
----------------
no bracket here


================
Comment at: clang/test/CodeGenHLSL/validator_version.hlsl:10
+  return bar(a, b);
+}
----------------
no bracket here


================
Comment at: clang/unittests/Driver/ToolChainTest.cpp:536
+    DAL->append(A);
+  }
+  auto *TranslatedArgs =
----------------
here too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123884



More information about the cfe-commits mailing list