[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