mstorsjo marked an inline comment as done. mstorsjo added inline comments.
================ Comment at: lib/Driver/ToolChains/MSVC.cpp:493 C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found); + linkPath = TC.GetProgramPath("link.exe"); + } ---------------- rnk wrote: > amccarth wrote: > > The comment above explains one reason why we shouldn't use link.exe on the > > path. > > > > If it is an appropriate fallback, modify the comment or add another one > > here explaining why this is better than failing. I think you hit on it in > > the patch summary, but it should be captured in the code. > Right, and this code block is inside some crazy getenv check for > USE_PATH_LINK, so I think we don't want to do a path search here. Then again, > I bet someone added that because they wanted clang to just do a path search. > I guess, there's your workaround. Sorry for the confusion - the getenv USE_PATH_LINK thingie there isn't present upstream, it's my other initial hack to get around this issue; I should have reordered the patches before making this diff. So that getenv hack is an ugly but convenient (for me) way around the issue. Another alternative is to use `-fuse-ld=link.exe` (which happens to miss the `Linker.equals_lower("link")` check). Prior to switching to lld, how did chromium do this? I presume the cross compilation setup did exist already before lld. Although in practice I guess most people don't use (clang-)cl for linking but just invoke `link` directly so maybe it didn't matter at all. Another approach (which also works fine for me) is to look for `$(dirname $(which cl))/link.exe`. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60094/new/ https://reviews.llvm.org/D60094 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits