[PATCH] D121992: [Clang] [Driver] Add option to set alternative toolchain path

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added a comment.

Hi,

> Why is --overlay-platform-toolchain added instead of using -isystem and -L?
>
> The functionality overlaps with -B. Unsure why introduce a new mechanism.

We may want to use an extra toolchain like the Advance Toolchain (https://github.com/advancetoolchain/advance-toolchain) which includes Glibc/GCC/GDB/LD/etc. but is not a complete OS distribution. So we should not simply change `sysroot` here.

Using `-isystem` and `-L` is okay in principle, but (1) it breaks expected include order (`-isystem` just inserts it in the top); (2) we have to manually insert many `-isystem` paths; (3) we want to reuse the logic in clang driver code. `-B` changes search path of crt runtime files, but include/library/dynamic linker paths are the same.

What this option does is to insert the extra toolchain in all search paths but with higher priority than system default.



================
Comment at: clang/include/clang/Driver/Options.td:4184
+def _overlay_platform_toolchain_EQ : Joined<["--"], "overlay-platform-toolchain=">;
+def _overlay_platform_toolchain : Separate<["--"], "overlay-platform-toolchain">, Alias<_overlay_platform_toolchain_EQ>;
 def _param : Separate<["--"], "param">, Group<CompileOnly_Group>;
----------------
MaskRay wrote:
> Separate-form driver options are not conventional. New driver options should just avoid them.
Thanks for the reminder!


================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1870
 
+  if (const Arg *X = Args.getLastArg(
+          clang::driver::options::OPT__overlay_platform_toolchain_EQ))
----------------
MaskRay wrote:
> Why was this  rule added?
Linker and other paths relies on location of specified GCC toolchain. And the toolchain specified by `overlay-platform-toolchain` is expected to have GCC installation included. But for sure, it has lower priority than `gcc-toolchain`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121992/new/

https://reviews.llvm.org/D121992