barannikov88 added a comment. In D131225#3706207 <https://reviews.llvm.org/D131225#3706207>, @abidh wrote:
> I think this patch is moving the things in the right direction. But please > wait for some other reviewers to chime in. It seems quite opposite to me. ================ Comment at: clang/include/clang/Driver/ToolChain.h:384 + /// IsBareMetal - Does this tool chain is a baremetal target. + static bool IsBareMetal(const llvm::Triple &); ---------------- Is this a correct sentence? (My English is poor.) ================ Comment at: clang/include/clang/Driver/ToolChain.h:388 + /// IsRISCVBareMetal - Does this tool chain is a riscv baremetal target. + static bool IsRISCVBareMetal(const llvm::Triple &); + ---------------- The ToolChain class is an interface class. It is strange to see such kind of methods here. `IsBareMetal` should at least be virtual and overridden in concrete implementation of baremetal toolchains. `IsRISCVBareMetal` should not be here at all. What was wrong with the previous implementation that made you move the methods here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131225/new/ https://reviews.llvm.org/D131225 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
