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

Thu Mar 31 00:49:43 PDT 2022

qiucf added a comment.

In D121992#3418443 <https://reviews.llvm.org/D121992#3418443>, @MaskRay wrote:

> To add more why I think the current semantics may need more discussion before we quickly commit to such a driver option:
>
> - interaction with --dyld-prefix: --sysroot does not affect the fallback --dyld-prefix. It seems even less appropriate for --overlay-platform-toolchain to affect the fallback --dyld-prefix.
>
> If you intend to overlay ld.so, you'll necessarily overlay libc, then --sysroot seems just unneeded at all.
>
> - interaction with --gcc-toolchain: I am a bit unclear we still want the tricky GCC installation detection after --overlay-platform-toolchain is specified. Do you propose that both will add include and library search paths?
> - -B: in gcc, when -B prefix specifies a directory, GCC adds $prefix/include to the include search directory. Clang does not do this right now. I think adding it may be an alternative approach to introducing the new option.
>
> The currently picked rules may be suitable for https://github.com/advancetoolchain/advance-toolchain, but could be arbitrary for many other use cases.
> Have you considered putting some options into a configuration file <https://clang.llvm.org/docs/UsersManual.html#configuration-files> and using `--config`?
>
> I understand that you probably have some short-term goal to make somethings done, but as I mentioned, there might be some process issue here.
> This significant new feature very quickly landed without other driver folks possibly had a chance to chime in.
> (FWIW I decided to subscribe all `clang/lib/Driver` patches since I care about this area.)
> I very rarely do this but I think it is probably cleaner to revert this patch and discuss it more carefully. I am happy to help you achieve your goal. It's possible that we may still need a driver option.

Thanks for further information. I agree that we should not change to break the consistent behavior in such short period of time. It's reasonable for `--gcc-toolchain` to not add `include` into path (since driver only expects it to have GCC). I also saw GCC's different behavior on `-B`. We may need to fix it, but that's beyond this patch's scope.

It's okay to me to compose 'orthogonal' options into config file when available. Anyway, I'd like to revert this commit since there're comments not addressed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121992