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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
