rnk 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");
+ }
----------------
mstorsjo wrote:
> 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`.
I think it's worth implementing `$(dirname $(which cl))/link.exe` because
/usr/bin/link exists on Linux and in many Unix shell environments for Windows.
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
