MaskRay added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:85
+ auto linkerIs = [Exec](const char *name) {
+ return llvm::sys::path::filename(Exec).equals_insensitive(name) ||
+ llvm::sys::path::stem(Exec).equals_insensitive(name);
----------------
ADKaster wrote:
> MaskRay wrote:
> > Avoid `equals_insensitive`. It's only recommended in some places for
> > Windows.
> What is the recommendation for this use case instead? This is the same
> pattern that is used by the Fuchsia driver.
> https://github.com/llvm/llvm-project/blob/01e3393b94d194ee99e57f23f9c08160cef94753/clang/lib/Driver/ToolChains/Fuchsia.cpp#L59-L61
>
> though it looks like Fuchsia used that pattern back when it was called
> `equals_lower()`.
> https://github.com/llvm/llvm-project/commit/3e199ecdadf7b546054c5a5820d1678f1e83c821
I think Fuchsia should not use `equals_lower`. There is never a case that we'd
use something like `ld.LLD`.
================
Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:127
+ Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
+ Args.AddAllArgs(CmdArgs, options::OPT_e);
+ Args.AddAllArgs(CmdArgs, options::OPT_s);
----------------
`OPT_e` has the LinkerInput flag and is rendered by AddLinkerInputs. Remove
`Args.AddAllArgs(CmdArgs, options::OPT_e);` to avoid duplicate `-e`.
================
Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:130
+ Args.AddAllArgs(CmdArgs, options::OPT_t);
+ Args.AddAllArgs(CmdArgs, options::OPT_Z_Flag);
+ Args.AddAllArgs(CmdArgs, options::OPT_r);
----------------
gcc doesn't recognize `-Z`. I think it is an early (200x) mistake that some
targets render `-Z`. Drop it.
================
Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:143
+ const SanitizerArgs &Sanitize = TC.getSanitizerArgs(Args);
+ if (Sanitize.needsUbsanRt())
+ CmdArgs.push_back("-lubsan");
----------------
I wonder why you don't use the common `addSanitizerRuntimes`. ubsan runtime is
normally `libclang_rt.ubsan_standalone*`
================
Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:195
+ getProgramPaths().push_back(getDriver().Dir);
+ getFilePaths().push_back(getDriver().SysRoot + "/usr/local/lib");
+ getFilePaths().push_back(getDriver().SysRoot + "/usr/lib");
----------------
`/usr/local/lib` should not be in the default search path.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154396/new/
https://reviews.llvm.org/D154396
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
