python3kgae marked 2 inline comments as done.
python3kgae added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:170
+
+Tool *clang::driver::toolchains::HLSLToolChain::getTool(
+    Action::ActionClass AC) const {
----------------
jhuber6 wrote:
> python3kgae wrote:
> > jhuber6 wrote:
> > > 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.
> > This is following pattern of MachO::getTool for VerifyDebug.
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Darwin.cpp#L1078
> > 
> > I can add getValidator to HLSLToolChain if that helps.
> > 
> Yeah, I guess it's a style thing. Personally I don't mind having everything 
> in one place because you need to handle the ActionClass there anyway. But 
> it's not a huge deal. I'll accept either.
Thanks. I'll keep it as is then.
The validator is only for HLSL now.
If other ToolChain uses it, I'll move it to ToolChain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141705

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to