krisb added a comment. Thank you! LGTM, except some minor nits below.
================ Comment at: clang/lib/Driver/ToolChains/MSP430.cpp:154 + SmallString<128> MultilibInclude(GCCInstallation.getInstallPath()); + llvm::sys::path::append(MultilibInclude, "include"); ---------------- I'd guard this by `if (GCCInstallation.isValid())` to avoid adding include directories with relative paths if `GCCInstallation.getInstallPath()` is empty. ================ Comment at: clang/lib/Driver/ToolChains/MSP430.cpp:239 + Arg *SspFlag = Args.getLastArg( + options::OPT_fno_stack_protector, options::OPT_fstack_protector, + options::OPT_fstack_protector_all, options::OPT_fstack_protector_strong); ---------------- Is the check for `fno-stack-protector` necessary here? Looks as the checks for 'positive' options should be enough to do the trick. ================ Comment at: clang/test/Driver/msp430-toolchain.c:5 + +// Test for include paths (cannot use -###) + ---------------- This way looks okay to me, but I believe you should be able to check cc1 command (though -###) for `-internal-isystem`, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81676/new/ https://reviews.llvm.org/D81676 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
