Typically clang finds Visual Studio by the user explicitly setting up a Visual Studio environment via vcvarsall. But we still try to behave intelligently and fallback to different methods of finding Visual Studio when this is not done. This patch improves various fallback codepaths to make Visual Studio locating more robust. Specifically, this patch: * Adds support for searching environment variables for VS 12.0 * Correctly locates include folders for Windows SDK 8.x (this was previously broken, and would cause clang to error). Windows SDK 8.x segregates its includes into \shared, \um, and \winrt. * Prefers locating link.exe in the same location as cl.exe. This is helpful in case another link.exe is in the path earlier than Visual Studio (e.g. GnuWin32) * Minor cleanup in the registry reading code to make it more robust in the presence of long pathnames.
Fix an issue with a stack smash.
There is still an outstanding issue remaining after this patch, which is that link cannot locate libraries. I think we can address this in a followup patch by adding some library paths to the link line similar to how we already do for system include paths that we pass to cl.
I'm not a big fan of our code for finding a msvc installation when vcvars.bat hasn't been run. I think that if the user wants to run clang-cl as a drop-in replacement for cl.exe, it should be dropped into the same environment.
But since we have it, we might as well make it work better. The TODO sounds reasonable, as long as it still checks PATH first, before going off and looking in other places.
Also, will cl.exe and link.exe etc. actually run if they're not on PATH? I recall having problems executing them without being on PATH because they failed to load some dll.
Style nit: FIXME without name is more common than TODO, and the |text| syntax isn't used.
Consider the case where you have a script which invokes clang many times with different sets of options each time. Sometimes m32, sometimes m64, for example. In this case it's cumbersome and difficult for the script to configure the environment beforehand, because there's no way to run a batch file "in process". If you try to run vcvars from a script it will just modify the environment of the shell process that is spawned to run the batch file.
I'm not a huge fan of it either though, and I actually think an ideal solution is to allow clang to accept a command line option that will just tell it the path to the root of the Visual Studio installation to use, and clang can just set up a consistent environment prior to invoking cl or link.
Heh. Probably not the best name in the world. I'll fix that.
Also prefer finding the clang cl.exe fallback location by using the visual studio directory which is being used for includes instead of looking in the PATH first.
VC system headers are tied to a particular cl.exe and should not be mixed. Previously, we were looking for cl.exe in the PATH, and using a different algorithm to find the VC include directories. Now, the two methods use the same algorithm. Or at the very least, they try to. This also makes it possible for clang to fallback to cl even when cl is not in the PATH.
Using getVisualStudioDir() for both headers and binaries seems like a big win!
Shouldn't it be searching for FallbackName rather than hard-coding cl.exe?
I think VSDir can be passed to can_execute directly and the Twine gets constructed implicitly.
It seems the code above is now doing what this fixme is suggesting :)
I realize you didn't add them, but again I think we can drop the Twines. (hopefully)
Maybe we should repurpose FindFallback to a function that finds vs executables and just pass in link.exe instead of doing this dance? We could then cut down on the comment too, because it would be obvious that the link.exe and cl.exe are from the same visual studio.