[MSVC] If unable to find link.exe from a MSVC installation, look for link.exe next to cl.exe

[mstorsjo]

Summary

Previously, if the MSVC installation isn't detected properly, clang will later just fail to execute link.exe.

This improves using clang in msvc mode on linux, where one intentionally might not want to point clang to the MSVC installation itself (which isn't executable as such), but where a link.exe named wine wrapper is available in the path next to a cl.exe named wine wrapper.

[amccarth]

lib/Driver/ToolChains/MSVC.cpp

492

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.

[rnk]

lib/Driver/ToolChains/MSVC.cpp

492

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.

[mstorsjo]

lib/Driver/ToolChains/MSVC.cpp

492

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.

[mstorsjo]

FWIW, to add a bit of context about why (since normally, few people actually do link via the (clang-)cl interface); CMake does during some of its tests.

I'd like to bring it to a point so that CC=clang-cl CXX=clang-cl cmake ... would work, while I currently either need my env var hack, or e.g. CC="clang-cl -fuse-ld=link.exe" ... (or -fuse-ld=lld-link). Doing that is not hard, but it's a bit of a nuisance as I tend to forget it.

[rnk]

lib/Driver/ToolChains/MSVC.cpp

492

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.

[mstorsjo]

Updated to not look blindly for link(.exe) in the PATH, but look for cl.exe and look for link.exe next to it.

[mstorsjo]

Removed other local experiments from the patch context.

[rnk]

lgtm

lib/Driver/ToolChains/MSVC.cpp

if (Linker.equals_lower("lld"))
     Linker = "lld-link";
 
   if (Linker.equals_lower("link")) {
     // If we're using the MSVC linker, it's not sufficient to just use link
     // from the program PATH, because other environments like GnuWin32 install
     // their own link.exe which may come first.
     linkPath = FindVisualStudioExecutable(TC, "link.exe");
 
-    if (!TC.FoundMSVCInstall() && !llvm::sys::fs::can_execute(linkPath))
+    if (!TC.FoundMSVCInstall() && !llvm::sys::fs::can_execute(linkPath)) {
+      llvm::SmallString<128> ClPath;
+      ClPath = TC.GetProgramPath("cl.exe");
+      if (llvm::sys::fs::can_execute(ClPath)) {
+        linkPath = llvm::sys::path::parent_path(ClPath);
+        llvm::sys::path::append(linkPath, "link.exe");
+        if (!llvm::sys::fs::can_execute(linkPath))
           C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
+      } else {
+        C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
+      }
+    }
 
 #ifdef _WIN32
     // When cross-compiling with VS2017 or newer, link.exe expects to have
     // its containing bin directory at the top of PATH, followed by the
     // native target bin directory.
     // e.g. when compiling for x86 on an x64 host, PATH should start with:
     // /bin/Hostx64/x86;/bin/Hostx64/x64
     // This doesn't attempt to handle ToolsetLayout::DevDivInternal.