[PATCH] Make a good guess about where MSVC and Windows SDK libraries are for linking.

Reid Kleckner rnk at google.com
Tue Oct 21 16:09:16 PDT 2014


================
Comment at: lib/Driver/Tools.cpp:7853
@@ +7852,3 @@
+    // If the VC environment hasn't been configured (perhaps because the user
+    // did not run vcvarsall, try to build a consistent link environment.  If
+    // the environment variable is set however, assume the user knows what he's
----------------
Close the parenthetical before the comma

================
Comment at: lib/Driver/Tools.cpp:7856
@@ +7855,3 @@
+    // doing.
+    std::string visualStudioDir;
+    auto &WTC = static_cast<const toolchains::Windows &>(getToolChain());
----------------
The style guide says to use upper case local variables. Some code is inconsistent, but this code uses mostly upper case locals, so let's lean that way here.

================
Comment at: lib/Driver/Tools.cpp:7857
@@ +7856,3 @@
+    std::string visualStudioDir;
+    auto &WTC = static_cast<const toolchains::Windows &>(getToolChain());
+    if (WTC.getVisualStudioInstallDir(visualStudioDir)) {
----------------
Hm, that seems to be a common pattern elsewhere, so OK.

Our style guide currently says to add const before auto if you know it's const.

================
Comment at: lib/Driver/Tools.cpp:7861
@@ +7860,3 @@
+      llvm::sys::path::append(VSDir, "VC\\lib");
+      switch (WTC.getArch()) {
+      case llvm::Triple::x86:
----------------
Can we shorten this up a bunch with a helper that takes the arch and returns a string for where to look?

I think this would be cleaner if you do a single call to append, like so:

  StringRef SubDir = getVCArchSubDir(WTC.getArch());
  llvm::sys::path::append(VSDir, "VC", "lib", SubDir);

Appending the empty string is a no-op, so the basic x86 case can return "", but the ppc case will have to return false or something else weird.

I expect this helper will also apply to VC/bin in the other patch.

It's also nice to avoid hardcoded backslashes so that we can eventually support cross-compilation.

================
Comment at: lib/Driver/Tools.cpp:7878
@@ +7877,3 @@
+
+    std::string windowsSdkLibPath;
+    if (WTC.getWindowsSDKLibraryPath(windowsSdkLibPath))
----------------
`WindowsSdkLibPath`

================
Comment at: lib/Driver/WindowsToolChain.cpp:230
@@ +229,3 @@
+  if (sdkMajor <= 7) {
+    switch (getArch()) {
+    // In Windows SDK 7.x, x86 libraries are directly in the Lib folder.
----------------
A similar helper like `getWindowsSDKArchSubDir(WTC.getArch(), sdkMajor)` might help here.

================
Comment at: lib/Driver/WindowsToolChain.cpp:251
@@ +250,3 @@
+    bool found = false;
+    for (size_t i = 0; i < llvm::array_lengthof(tests); ++i) {
+      llvm::SmallString<128> testPath(libPath);
----------------
You can use a range for loop over the local array.

http://reviews.llvm.org/D5873






More information about the cfe-commits mailing list