manojgupta added inline comments.
================ Comment at: clang/include/clang/Driver/ToolChain.h:384 + /// IsBareMetal - Does this tool chain is a baremetal target. + static bool IsBareMetal(const llvm::Triple &); ---------------- barannikov88 wrote: > Is this a correct sentence? (My English is poor.) > I'll fix it. Somehow I forgot to check them. ================ 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 &); + ---------------- barannikov88 wrote: > 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? There is a need to check for IsBareMetal and variants. They can't be made virtual since the checks need to happen before instantiating ToolChain class. I think moving them to Triple class (Triple.h) is a clearer option. 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
