Loosen MSVC 2017 path requirements

[dmajor]

Summary

Mozilla's build machines are currently applying this patch locally, but I thought I'd offer it upstream because it should be pretty harmless.

clang-cl has some sanity checks to make sure that the cl.exe it finds is actually the Microsoft compiler and not something else. The code expects the path to look like:
...\VC\Tools\MSVC\14.11.25503\bin\HostX64\x64\cl.exe

It doesn't work on Mozilla's build automation where we have a fake Visual Studio installation that looks like: c:\vs2017\VC\bin\HostX64\x64\cl.exe

This patch reduces the path requirement to ...\bin\Host*\*\cl.exe.

[rnk]

I haven't dug into this code to really understand if this is right and won't change our version detection logic, but yes, broadly I believe we should just consult PATH, find a cl.exe, and check its version. I'd like to reduce this path validation to a minimum.

[hans]

I think the patch is fine, but Zach should probably sign off on it.

[zturner]

I'm not suuuper opposed, but at the same time if this code is bothering people (and it is, consistently), I don't changing the requirements from "confusing rule A" to "confusing rule B" is going to solve the long term burden that people keep running into.

Not asking you to do this work, but my ideal solution is probably to teach clang-cl to recognize 3 new environment variables.

CLANGCL_MSVC_BIN - Where to look for cl.exe, and link.exe. Under no circumstances do we consult PATH or anything else. This is only used when we need to fallback to cl (rarely, anymore) or when the compiler needs to invoke the linker. But! At the same time we make -fuse-ld=lld the default. We only do something else if the user specifies -fuse-ld=link, and in that case it uses CLANGCL_MSVC_BIN (or if you specified -fuse-ld=<abs path> then the env var isn't needed).

CLANGCL_WINSDK - Points to the root of the Windows SDK. The folder here should have a "standard" layout so that it least pretends to be an installation, so that we can find the right lib directory when cross-compiling.

CLANGCL_MSVCRT - Same as before, but for the CRT.

I would honestly like to delete just about 100% of this stuff about finding MSVC. We should just use exactly what we're told to use and nothing else. Simple, easy to understand, and easy to explain.

Anyway, I'm just venting. If rnk@ wants to lgtm this, I'm fine.

[dmajor]

Anyway, I'm just venting. If rnk@ wants to lgtm this, I'm fine.

@rnk, any objection to this patch?

lib/Driver/ToolChains/MSVC.cpp

for (llvm::StringRef PathEntry : PathEntries) {
           return true;
         }
 
       } else {
         // This could be a new (>=VS2017) toolchain. If it is, we should find
         // path components with these prefixes when walking backwards through
         // the path.
         // Note: empty strings match anything.
-        llvm::StringRef ExpectedPrefixes[] = {"",     "Host",  "bin", "",
-                                              "MSVC", "Tools", "VC"};
+        llvm::StringRef ExpectedPrefixes[] = {"", "Host", "bin"};
 
         auto It = llvm::sys::path::rbegin(PathEntry);
         auto End = llvm::sys::path::rend(PathEntry);
         for (llvm::StringRef Prefix : ExpectedPrefixes) {
           if (It == End)
             goto NotAToolChain;
           if (!It->startswith(Prefix))
             goto NotAToolChain;