[PATCH] Improve Windows toolchain support for non-standard environments.

Zachary Turner zturner at google.com
Fri Oct 17 13:06:38 PDT 2014


================
Comment at: lib/Driver/Tools.cpp:7817
@@ +7816,3 @@
+  // system includes and libraries from one VS installation, and use cl and link
+  // from another installation.
+  llvm::Optional<std::string> OptPath = llvm::sys::Process::GetEnv("PATH");
----------------
hans wrote:
> 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.

================
Comment at: lib/Driver/Tools.cpp:7930
@@ +7929,3 @@
+  // the path.
+  llvm::SmallString<128> smartPath(
+      FindFallback("cl.exe", C.getDriver().getClangProgramPath()));
----------------
hans wrote:
> smartPath?
Heh.  Probably not the best name in the world.  I'll fix that.

http://reviews.llvm.org/D5845






More information about the cfe-commits mailing list