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

Reply via email to