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

Hans Wennborg hans at chromium.org
Fri Oct 17 16:45:59 PDT 2014


Using getVisualStudioDir() for both headers and binaries seems like a big win!

================
Comment at: lib/Driver/Tools.cpp:7816
@@ +7815,3 @@
+    SmallString<128> VSDir(visualStudioDir);
+    llvm::sys::path::append(VSDir, "VC\\bin\\cl.exe");
+    if (llvm::sys::fs::can_execute(Twine(VSDir)))
----------------
Shouldn't it be searching for FallbackName rather than hard-coding cl.exe?

================
Comment at: lib/Driver/Tools.cpp:7817
@@ +7816,3 @@
+    llvm::sys::path::append(VSDir, "VC\\bin\\cl.exe");
+    if (llvm::sys::fs::can_execute(Twine(VSDir)))
+      return VSDir.str();
----------------
I think VSDir can be passed to can_execute directly and the Twine gets constructed implicitly.

================
Comment at: lib/Driver/Tools.cpp:7822
@@ +7821,3 @@
+  // FIXME: Maybe instead of checking the PATH this should tack "\bin" onto the
+  // end of toolchains::Windows::getVisualStudioDir().  This would make clang
+  // smarter in the case of running clang without first running vcvarsall, and
----------------
It seems the code above is now doing what this fixme is suggesting :)

================
Comment at: lib/Driver/Tools.cpp:7842
@@ +7841,3 @@
+    if (llvm::sys::fs::can_execute(Twine(FilePath)) &&
+        !llvm::sys::fs::equivalent(Twine(FilePath), ClangProgramPath))
+      return FilePath.str();
----------------
I realize you didn't add them, but again I think we can drop the Twines. (hopefully)

================
Comment at: lib/Driver/Tools.cpp:7938
@@ +7937,3 @@
+  llvm::SmallString<128> linkPath(FindFallback(
+      getToolChain(), "cl.exe", C.getDriver().getClangProgramPath()));
+  llvm::sys::path::remove_filename(linkPath);
----------------
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.

http://reviews.llvm.org/D5845






More information about the cfe-commits mailing list