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