[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added a comment.

In https://reviews.llvm.org/D28365#665408, @rnk wrote:

> Doesn't your fix mean that the tests will fail on a Windows machine that 
> doesn't have VS because LLVM was built with mingw? Usually in these 
> situations we provide some way to provide a fake toolchain.


I'm not sure. It should be the same behaviour as before, which is that it'll 
keep going regardless of whether it can find an MSVC instance. Was there some 
fake toolchain providing that I missed in the old code?


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Doesn't your fix mean that the tests will fail on a Windows machine that 
doesn't have VS because LLVM was built with mingw? Usually in these situations 
we provide some way to provide a fake toolchain.


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I had to revert this because it doesn't pass tests on Linux. Can you look into 
that and resubmit after fixing those test failures?


Repository:
  rL LLVM

https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL293923: [Driver] Updated for Visual Studio 2017 (authored by 
rnk).

Changed prior to commit:
  https://reviews.llvm.org/D28365?vs=86502=86865#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28365

Files:
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/lib/Driver/MSVCToolChain.cpp
  cfe/trunk/lib/Driver/ToolChains.h
  cfe/trunk/lib/Driver/Tools.cpp

Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
@@ -281,4 +281,8 @@
 def err_drv_unsupported_linker : Error<"unsupported value '%0' for -linker option">;
 def err_drv_defsym_invalid_format : Error<"defsym must be of the form: sym=value: %0">;
 def err_drv_defsym_invalid_symval : Error<"Value is not an integer: %0">;
+
+def err_drv_msvc_not_found : Error<
+  "unable to find a Visual Studio installation; "
+  "try running Clang from a developer command prompt">;
 }
Index: cfe/trunk/lib/Driver/MSVCToolChain.cpp
===
--- cfe/trunk/lib/Driver/MSVCToolChain.cpp
+++ cfe/trunk/lib/Driver/MSVCToolChain.cpp
@@ -23,16 +23,23 @@
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Host.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include 
 
-// Include the necessary headers to interface with the Windows registry and
-// environment.
 #if defined(LLVM_ON_WIN32)
-#define USE_WIN32
+  #define USE_WIN32
+
+  // FIXME: Make this configurable with cmake when the final version of the API
+  //has been released.
+  #if 0
+#define USE_VS_SETUP_CONFIG
+  #endif
 #endif
 
+// Include the necessary headers to interface with the Windows registry and
+// environment.
 #ifdef USE_WIN32
   #define WIN32_LEAN_AND_MEAN
   #define NOGDI
@@ -42,20 +49,265 @@
   #include 
 #endif
 
+// Include the headers needed for the setup config COM stuff and define
+// smart pointers for the interfaces we need.
+#ifdef USE_VS_SETUP_CONFIG
+  #include "clang/Basic/VirtualFileSystem.h"
+  #include "llvm/Support/COM.h"
+  #include 
+  #include 
+  _COM_SMARTPTR_TYPEDEF(ISetupConfiguration,  __uuidof(ISetupConfiguration));
+  _COM_SMARTPTR_TYPEDEF(ISetupConfiguration2, __uuidof(ISetupConfiguration2));
+  _COM_SMARTPTR_TYPEDEF(ISetupHelper, __uuidof(ISetupHelper));
+  _COM_SMARTPTR_TYPEDEF(IEnumSetupInstances,  __uuidof(IEnumSetupInstances));
+  _COM_SMARTPTR_TYPEDEF(ISetupInstance,   __uuidof(ISetupInstance));
+  _COM_SMARTPTR_TYPEDEF(ISetupInstance2,  __uuidof(ISetupInstance2));
+#endif
+
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
 using namespace clang;
 using namespace llvm::opt;
 
-MSVCToolChain::MSVCToolChain(const Driver , const llvm::Triple ,
+// Defined below.
+// Forward declare this so there aren't too many things above the constructor.
+static bool getSystemRegistryString(const char *keyPath, const char *valueName,
+std::string , std::string *phValue);
+
+// Check various environment variables to try and find a toolchain.
+static bool findVCToolChainViaEnvironment(std::string ,
+  bool ) {
+  // These variables are typically set by vcvarsall.bat
+  // when launching a developer command prompt.
+  if (llvm::Optional VCToolsInstallDir =
+  llvm::sys::Process::GetEnv("VCToolsInstallDir")) {
+// This is only set by newer Visual Studios, and it leads straight to
+// the toolchain directory.
+Path = std::move(*VCToolsInstallDir);
+IsVS2017OrNewer = true;
+return true;
+  }
+  if (llvm::Optional VCInstallDir =
+  llvm::sys::Process::GetEnv("VCINSTALLDIR")) {
+// If the previous variable isn't set but this one is, then we've found
+// an older Visual Studio. This variable is set by newer Visual Studios too,
+// so this check has to appear second.
+// In older Visual Studios, the VC directory is the toolchain.
+Path = std::move(*VCInstallDir);
+IsVS2017OrNewer = false;
+return true;
+  }
+
+  // We couldn't find any VC environment variables. Let's walk through PATH and
+  // see if it leads us to a VC toolchain bin directory. If it does, pick the
+  // first one that we find.
+  if (llvm::Optional PathEnv =
+  llvm::sys::Process::GetEnv("PATH")) {
+llvm::SmallVector PathEntries;
+llvm::StringRef(*PathEnv).split(PathEntries, llvm::sys::EnvPathSeparator);
+for (llvm::StringRef PathEntry : PathEntries) {
+  if (PathEntry.empty())
+continue;
+
+  llvm::SmallString<256> ExeTestPath;
+
+  // If cl.exe doesn't exist, then this definitely isn't a VC toolchain.
+  

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added a comment.

In https://reviews.llvm.org/D28365#664892, @rnk wrote:

> This is ready to land. Do you need someone to commit this?


I think so, yeah.


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

This is ready to land. Do you need someone to commit this?


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 86502.
hamzasood added a comment.

Added a FIXME comment about configuring use of the Setup Configuration API.
In its current form (RC3) the header is distributed in a nuget package, so it's 
installed per-project instead of being in a system location that can be 
automatically detected. This could change for the final release, so it probably 
isn't worth thinking about it too much yet.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10888,19 +10888,12 @@
 // making sure that whatever executable that's found is not a same-named exe
 // from clang itself to prevent clang from falling back to itself.
 static std::string FindVisualStudioExecutable(const ToolChain ,
-  const char *Exe,
-  const char *ClangProgramPath) {
+  const char *Exe) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10909,7 +10902,7 @@
 const ArgList ,
 const char *LinkingOutput) const {
   ArgStringList CmdArgs;
-  const ToolChain  = getToolChain();
+  auto  = static_cast(getToolChain());
 
   assert((Output.isFilename() || Output.isNothing()) && "invalid output");
   if (Output.isFilename())
@@ -10925,37 +10918,20 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
-const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + TC.getSubDirectoryPath(toolchains::MSVCToolChain
+   ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (TC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (TC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
-if (MSVC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
+if (TC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
   CmdArgs.push_back(
   Args.MakeArgString(std::string("-libpath:") + WindowsSdkLibPath));
   }
@@ -11079,8 +11055,7 @@
 // If we're using the MSVC linker, it's not sufficient to just use link
 // from the program PATH, because other environments like GnuWin32 install
 // their own link.exe which may come first.
-linkPath = FindVisualStudioExecutable(TC, "link.exe",
-  C.getDriver().getClangProgramPath());
+linkPath = FindVisualStudioExecutable(TC, "link.exe");
   } else {
 linkPath = Linker;
 llvm::sys::path::replace_extension(linkPath, "exe");
@@ -11213,9 +11188,7 @@
   Args.MakeArgString(std::string("/Fo") + Output.getFilename());
   CmdArgs.push_back(Fo);
 
-  const 

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: lib/Driver/MSVCToolChain.cpp:34
+  #if 0
+#define USE_VS_SETUP_CONFIG
+  #endif

hamzasood wrote:
> rnk wrote:
> > What are the outstanding issues preventing you from enabling this by 
> > default?
> Building on Win32 doesn't imply that you'll have the required header;  it 
> currently needs to be installed [[ 
> https://blogs.msdn.microsoft.com/heaths/2016/09/15/changes-to-visual-studio-15-setup/
>  | separately ]] via nuget. While it would be possible to have cmake create a 
> packages.config file when configuring a visual studio project, the API is 
> only at the RC stage and so the distribution method could potentially change 
> between now and the Visual Studio 2017 release (even if that's not a concern, 
> it's probably out of the scope of this patch anyway).
> Although the code looks useless sitting there ifdefed out, it could be useful 
> for someone eager enough to get the package themselves and run a custom Clang 
> build.
> In the meantime, Visual Studio 2017 installations can only be detected when 
> running in the correct developer command prompt, or by putting one of its 
> toolchain's bin directories at the top of PATH.
This patch looks good, so I don't want to block it, but can you add something 
like:
 // FIXME: Use cmake to auto-detect if the necessary headers exist.
I'm assuming we should test for  eventually.


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood marked 5 inline comments as done.
hamzasood added a comment.

Ping


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added inline comments.



Comment at: lib/Driver/MSVCToolChain.cpp:34
+  #if 0
+#define USE_VS_SETUP_CONFIG
+  #endif

rnk wrote:
> What are the outstanding issues preventing you from enabling this by default?
Building on Win32 doesn't imply that you'll have the required header;  it 
currently needs to be installed [[ 
https://blogs.msdn.microsoft.com/heaths/2016/09/15/changes-to-visual-studio-15-setup/
 | separately ]] via nuget. While it would be possible to have cmake create a 
packages.config file when configuring a visual studio project, the API is only 
at the RC stage and so the distribution method could potentially change between 
now and the Visual Studio 2017 release (even if that's not a concern, it's 
probably out of the scope of this patch anyway).
Although the code looks useless sitting there ifdefed out, it could be useful 
for someone eager enough to get the package themselves and run a custom Clang 
build.
In the meantime, Visual Studio 2017 installations can only be detected when 
running in the correct developer command prompt, or by putting one of its 
toolchain's bin directories at the top of PATH.


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 84446.
hamzasood added a comment.

Broke up findVCToolChainPath into a few smaller functions.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10809,19 +10809,12 @@
 // making sure that whatever executable that's found is not a same-named exe
 // from clang itself to prevent clang from falling back to itself.
 static std::string FindVisualStudioExecutable(const ToolChain ,
-  const char *Exe,
-  const char *ClangProgramPath) {
+  const char *Exe) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10830,7 +10823,7 @@
 const ArgList ,
 const char *LinkingOutput) const {
   ArgStringList CmdArgs;
-  const ToolChain  = getToolChain();
+  auto  = static_cast(getToolChain());
 
   assert((Output.isFilename() || Output.isNothing()) && "invalid output");
   if (Output.isFilename())
@@ -10846,37 +10839,20 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
-const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + TC.getSubDirectoryPath(toolchains::MSVCToolChain
+   ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (TC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (TC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
-if (MSVC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
+if (TC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
   CmdArgs.push_back(
   Args.MakeArgString(std::string("-libpath:") + WindowsSdkLibPath));
   }
@@ -10991,8 +10967,7 @@
 // If we're using the MSVC linker, it's not sufficient to just use link
 // from the program PATH, because other environments like GnuWin32 install
 // their own link.exe which may come first.
-linkPath = FindVisualStudioExecutable(TC, "link.exe",
-  C.getDriver().getClangProgramPath());
+linkPath = FindVisualStudioExecutable(TC, "link.exe");
   } else {
 linkPath = Linker;
 llvm::sys::path::replace_extension(linkPath, "exe");
@@ -11125,9 +11100,7 @@
   Args.MakeArgString(std::string("/Fo") + Output.getFilename());
   CmdArgs.push_back(Fo);
 
-  const Driver  = getToolChain().getDriver();
-  std::string Exec = FindVisualStudioExecutable(getToolChain(), "cl.exe",
-D.getClangProgramPath());
+  std::string Exec = FindVisualStudioExecutable(getToolChain(), "cl.exe");
   return llvm::make_unique(JA, 

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/Driver/MSVCToolChain.cpp:34
+  #if 0
+#define USE_VS_SETUP_CONFIG
+  #endif

What are the outstanding issues preventing you from enabling this by default?



Comment at: lib/Driver/MSVCToolChain.cpp:160
+
+NotAToolChain:
+  ;

I think it would be clearer if you separated out the PATH search into a 
separate helper. 



Comment at: lib/Driver/MSVCToolChain.cpp:263
+
+ConfigQueryUnsuccessful:
+  ;

Similarly, I think this should be a separate helper. It wouldn't need goto.


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 83943.
hamzasood added a comment.

Uploaded the correct diff this time (yes, really)...
Not sure how to delete the previous one, but it's very incorrect.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10807,19 +10807,12 @@
 // making sure that whatever executable that's found is not a same-named exe
 // from clang itself to prevent clang from falling back to itself.
 static std::string FindVisualStudioExecutable(const ToolChain ,
-  const char *Exe,
-  const char *ClangProgramPath) {
+  const char *Exe) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10828,7 +10821,7 @@
 const ArgList ,
 const char *LinkingOutput) const {
   ArgStringList CmdArgs;
-  const ToolChain  = getToolChain();
+  auto  = static_cast(getToolChain());
 
   assert((Output.isFilename() || Output.isNothing()) && "invalid output");
   if (Output.isFilename())
@@ -10844,37 +10837,20 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
-const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + TC.getSubDirectoryPath(toolchains::MSVCToolChain
+   ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (TC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (TC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
-if (MSVC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
+if (TC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
   CmdArgs.push_back(
   Args.MakeArgString(std::string("-libpath:") + WindowsSdkLibPath));
   }
@@ -10989,8 +10965,7 @@
 // If we're using the MSVC linker, it's not sufficient to just use link
 // from the program PATH, because other environments like GnuWin32 install
 // their own link.exe which may come first.
-linkPath = FindVisualStudioExecutable(TC, "link.exe",
-  C.getDriver().getClangProgramPath());
+linkPath = FindVisualStudioExecutable(TC, "link.exe");
   } else {
 linkPath = Linker;
 llvm::sys::path::replace_extension(linkPath, "exe");
@@ -11123,9 +11098,7 @@
   Args.MakeArgString(std::string("/Fo") + Output.getFilename());
   CmdArgs.push_back(Fo);
 
-  const Driver  = getToolChain().getDriver();
-  std::string Exec = FindVisualStudioExecutable(getToolChain(), "cl.exe",
-D.getClangProgramPath());
+  std::string Exec = 

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 83941.
hamzasood added a comment.

- Rephrased the diagnostic message (and fixed an embarrassing typo).
- Reverted the linker environment change for now; building for x86 with VS2017 
won't work in the meantime. I'll submit it for review separately after this one 
has (hopefully) been accepted, as it relies on a few functions introduced in 
this patch.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10807,19 +10807,12 @@
 // making sure that whatever executable that's found is not a same-named exe
 // from clang itself to prevent clang from falling back to itself.
 static std::string FindVisualStudioExecutable(const ToolChain ,
-  const char *Exe,
-  const char *ClangProgramPath) {
+  const char *Exe) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10828,7 +10821,7 @@
 const ArgList ,
 const char *LinkingOutput) const {
   ArgStringList CmdArgs;
-  const ToolChain  = getToolChain();
+  auto  = static_cast(getToolChain());
 
   assert((Output.isFilename() || Output.isNothing()) && "invalid output");
   if (Output.isFilename())
@@ -10844,37 +10837,20 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
-const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + TC.getSubDirectoryPath(toolchains::MSVCToolChain
+   ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (TC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (TC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
-if (MSVC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
+if (TC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
   CmdArgs.push_back(
   Args.MakeArgString(std::string("-libpath:") + WindowsSdkLibPath));
   }
@@ -10989,8 +10965,7 @@
 // If we're using the MSVC linker, it's not sufficient to just use link
 // from the program PATH, because other environments like GnuWin32 install
 // their own link.exe which may come first.
-linkPath = FindVisualStudioExecutable(TC, "link.exe",
-  C.getDriver().getClangProgramPath());
+linkPath = FindVisualStudioExecutable(TC, "link.exe");
   } else {
 linkPath = Linker;
 llvm::sys::path::replace_extension(linkPath, "exe");
@@ -11123,9 +11098,7 @@
   Args.MakeArgString(std::string("/Fo") + Output.getFilename());
   CmdArgs.push_back(Fo);
 
-  const Driver  = 

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D28365#640868, @hamzasood wrote:

> Ha, good point. Does that include the environment stuff in Command too or 
> just the linker?


Yes, please make the Command environment changes as part of a separate patch 
with the linker environment changes.


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added a comment.

In https://reviews.llvm.org/D28365#640859, @rnk wrote:

> In https://reviews.llvm.org/D28365#640825, @hamzasood wrote:
>
> > In https://reviews.llvm.org/D28365#640775, @rnk wrote:
> >
> > > Can we revert the linker environment change from this patch? It'll be 
> > > easier to review separately.
> >
> >
> > Sure. But that'll break compiling for x86 on Visual Studio 2017, is that 
> > okay?
>
>
> Yeah, that's fine, you can't break what doesn't work yet. :)


Ha, good point. Does that include the environment stuff in Command too or just 
the linker?


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D28365#640825, @hamzasood wrote:

> In https://reviews.llvm.org/D28365#640775, @rnk wrote:
>
> > Can we revert the linker environment change from this patch? It'll be 
> > easier to review separately.
>
>
> Sure. But that'll break compiling for x86 on Visual Studio 2017, is that okay?


Yeah, that's fine, you can't break what doesn't work yet. :)


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added a comment.

In https://reviews.llvm.org/D28365#640775, @rnk wrote:

> Can we revert the linker environment change from this patch? It'll be easier 
> to review separately.


Sure. But that'll break compiling for x86 on Visual Studio 2017, is that okay?


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D28365#639253, @hamzasood wrote:

> - Added an option to set the environment in a clang::driver::Command, which 
> makes the environment modifying introduced in the last update a bit more 
> reliable.
>
>   @rnk I looked into using the new MSVC toolchain layout to get a version 
> number without opening an exe, but it doesn't look like it'll be possible. 
> The version number in the toolchain path is the MSVC version number (e.g. 
> Visual Studio 2015 ships with MSVC 14). The version numbers that Clang use 
> are the compiler version numbers (e.g. cl.exe v19 for Visual Studio 2015). As 
> far as I'm aware, there's no mapping between the two.


True, you're right. It's definitely out of scope for this change anyway.

---

Can we revert the linker environment change from this patch? It'll be easier to 
review separately.




Comment at: include/clang/Basic/DiagnosticDriverKinds.td:282
+def err_drv_msvc_not_found : Error<
+  "unable to find a Visual Studio installation. "
+  "Try re-running Clang from a devleoper command prompt">;

It would be nice if we had guidelines on writing clang diagnostics somewhere. I 
think they are supposed to be sentence fragments, and we typically add another 
sentence fragment with a semi-colon. I'd reword it like this for consistency:
  unable to find a Visual Studio installation; try running Clang from a 
developer command prompt



Comment at: include/clang/Basic/DiagnosticDriverKinds.td:283
+  "unable to find a Visual Studio installation. "
+  "Try re-running Clang from a devleoper command prompt">;
 }

typo on developer



Comment at: lib/Driver/Job.cpp:306-308
+  // Convert the environment vector into a vector of char pointers so we can
+  // get it as char**, as required by llvm::sys::ExecuteAndWait.
+  // SmallString::c_str isn't const, hence the const_cast.

Let's not do this, let's store a `std::vector`. We can get the 
right lifetime with `Args.MakeArgString`.


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added a comment.

In https://reviews.llvm.org/D28365#639260, @awson wrote:

> It's weird th[[ URL | name ]]at cl.exe command-line compiler reports version 
> 19.10.24629 and lives in the 14.10.24629 directory (only first 2 digits are 
> different).
>
> Moreover, in their explanation blogpost 
> 
>  they claim that //There’s a subdirectory in the MSVC directory with a 
> compiler version number. For VS 2017 RC, that version number is 
> 14.10.24629.//, i.e., they name this 14.10.24629 a "compiler version".


I'll admit I'm also slightly confused by the difference and maybe someone more 
familiar with the matter can correct me, but from what I understand: v14 is the 
toolchain version and v19 is the cl.exe version (i.e. specifically the compiler 
version). Wikipedia has a useful table 
 showing cl.exe 
version numbers over the years.  I've got no idea why they're kept separate, 
probably historical reasons.
As for the blog post, I can only assume that it's a typo and they meant to say 
"compiler tools version".


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Kyrill Briantsev via Phabricator via cfe-commits]

awson added a comment.

It's weird that cl.exe command-line compiler reports version 19.10.24629 and 
lives in the 14.10.24629 directory (only first 2 digits are different).

Moreover, in their explanation blogpost 

 they claim that //There’s a subdirectory in the MSVC directory with a compiler 
version number. For VS 2017 RC, that version number is 14.10.24629.//, i.e., 
they name this 14.10.24629 a "compiler version".


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 83555.
hamzasood added a comment.

- Added an option to set the environment in a clang::driver::Command, which 
makes the environment modifying introduced in the last update a bit more 
reliable.

@rnk I looked into using the new MSVC toolchain layout to get a version number 
without opening an exe, but it doesn't look like it'll be possible. The version 
number in the toolchain path is the MSVC version number (e.g. Visual Studio 
2015 ships with MSVC 14). The version numbers that Clang use are the compiler 
version numbers (e.g. cl.exe v19 for Visual Studio 2015). As far as I'm aware, 
there's no mapping between the two.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Job.h
  lib/Driver/Job.cpp
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -48,6 +48,15 @@
 #include  // For getuid().
 #endif
 
+#ifdef LLVM_ON_WIN32
+  #define WIN32_LEAN_AND_MEAN
+  #include 
+
+  // Undefine this macro so we can call the ANSI version of the function.
+  #undef GetEnvironmentStrings
+  #define GetEnvironmentStringsA GetEnvironmentStrings
+#endif
+
 using namespace clang::driver;
 using namespace clang::driver::tools;
 using namespace clang;
@@ -10806,19 +10815,12 @@
 // making sure that whatever executable that's found is not a same-named exe
 // from clang itself to prevent clang from falling back to itself.
 static std::string FindVisualStudioExecutable(const ToolChain ,
-  const char *Exe,
-  const char *ClangProgramPath) {
+  const char *Exe) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10827,7 +10829,7 @@
 const ArgList ,
 const char *LinkingOutput) const {
   ArgStringList CmdArgs;
-  const ToolChain  = getToolChain();
+  auto  = static_cast(getToolChain());
 
   assert((Output.isFilename() || Output.isNothing()) && "invalid output");
   if (Output.isFilename())
@@ -10843,37 +10845,20 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
-const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + TC.getSubDirectoryPath(toolchains::MSVCToolChain
+   ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (TC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (TC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
-if (MSVC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
+if (TC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
   CmdArgs.push_back(
   Args.MakeArgString(std::string("-libpath:") 

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 83473.
hamzasood added a comment.

- Modify the environment in some cases in buildLinker to keep link.exe happy.

Thanks @rnk for taking the time to look at this. As suggested:

- Replaced the confusing unique_ptr usage with a struct.
- Renamed llvmArchToSubDirectoryName to llvmArchToWindowsSDKArch.
- Replaced the nested function in getSubDirectoryPath with a static helper.

With regard to _set_com_error_handler, it's a documented 
 function so 
it is supported. It's not part of the actual COM API, just a wrapper library 
that sits on top of it. That library is a header only library with inline 
functions, so you can see how it's used. It checks the HRESULTs before 
returning them and if the result isn't successful then it calls the error 
handler; the original HRESULT still gets returned afterwards. You can also see 
from the headers that it isn't being relied on to process any kind of internal 
error, so it should be perfectly safe to block out for a bit.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10809,16 +10809,10 @@
   const char *Exe,
   const char *ClangProgramPath) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10843,33 +10837,17 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
 const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (MSVC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1155,6 +1155,15 @@
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;
 
+  enum class SubDirectoryType {
+Bin,
+Include,
+Lib,
+  };
+  std::string getSubDirectoryPath(SubDirectoryType Type) const;
+  std::string getSubDirectoryPath(SubDirectoryType Type,
+  llvm::Triple::ArchType TargetArch) const;
+
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
@@ -1163,19 +1172,12 @@
   llvm::opt::ArgStringList ) const override;
 
   void AddCudaIncludeArgs(const llvm::opt::ArgList ,
-  llvm::opt::ArgStringList ) const 

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

The new MSVC layout suggests to me that we should look try looking at the path 
of cl.exe before we open the exe and try to check its version. We'd change this 
code to inspect the path before looking in the exe:

  VersionTuple MSVCToolChain::computeMSVCVersion(const Driver *D,
 const ArgList ) const {
bool IsWindowsMSVC = getTriple().isWindowsMSVCEnvironment();
VersionTuple MSVT = ToolChain::computeMSVCVersion(D, Args);
if (MSVT.empty()) MSVT = getMSVCVersionFromTriple();
if (MSVT.empty() && IsWindowsMSVC) MSVT = getMSVCVersionFromExe();
if (MSVT.empty() &&
Args.hasFlag(options::OPT_fms_extensions, 
options::OPT_fno_ms_extensions,
 IsWindowsMSVC)) {
  // -fms-compatibility-version=18.00 is default.
  // FIXME: Consider bumping this to 19 (MSVC2015) soon.
  MSVT = VersionTuple(18);
}
return MSVT;
  }

That can definitely be a separate change, though.




Comment at: lib/Driver/MSVCToolChain.cpp:76
+// Attempts to find the "best" usable VC toolchain.
+static bool findVCToolChainPath(std::string , bool ) {
+  // Check the environment first, since that's probably the user telling us

Thanks for rewriting this, the logic here seems much clearer.



Comment at: lib/Driver/MSVCToolChain.cpp:186
+// this scope.
+_set_com_error_handler([](HRESULT, IErrorInfo *) { });
+std::unique_ptr

This is a very surprising use of unique_ptr. I think it would be clearer 
written this way:
  struct RestoreCOMErrorHandler {
~RestoreCOMErrorHandler() { _set_com_error_handler(_com_raise_error); }
  } COMErrorRestorer;



Comment at: lib/Driver/MSVCToolChain.cpp:352
+// to the corresponding subdirectory name.
+static const char *llvmArchToSubDirectoryName(llvm::Triple::ArchType Arch) {
+  using ArchType = llvm::Triple::ArchType;

Think there's a better name for this than "SubDirectoryName"? It sounds like 
these are common to the Win8+ SDk and the VC2017+ tools. We could still call it 
the "WindowsSDKArch", since that's the package that initially adopted this 
naming convention.



Comment at: lib/Driver/MSVCToolChain.cpp:370
+std::string MSVCToolChain::getSubDirectoryPath(SubDirectoryType Type) const {
+  auto llvmArchToLegacyVCSubDirectoryName =
+  [](llvm::Triple::ArchType Arch) -> const char * {

For consistency, please make this a static helper like the helper above.


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 83325.
hamzasood added a comment.

Improved the code slightly.
Sorry for the spam everyone, this is definitely the one.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10809,16 +10809,10 @@
   const char *Exe,
   const char *ClangProgramPath) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10843,33 +10837,17 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
 const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (MSVC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1155,6 +1155,13 @@
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;
 
+  enum class SubDirectoryType {
+Bin,
+Include,
+Lib,
+  };
+  std::string getSubDirectoryPath(SubDirectoryType Type) const;
+
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
@@ -1163,19 +1170,12 @@
   llvm::opt::ArgStringList ) const override;
 
   void AddCudaIncludeArgs(const llvm::opt::ArgList ,
-  llvm::opt::ArgStringList ) const override;
+  llvm::opt::ArgStringList ) const override;
 
-  bool getWindowsSDKDir(std::string , int ,
-std::string ,
-std::string ) const;
   bool getWindowsSDKLibraryPath(std::string ) const;
   /// \brief Check if Universal CRT should be used if available
-  bool useUniversalCRT(std::string ) const;
-  bool getUniversalCRTSdkDir(std::string , std::string ) const;
+  bool useUniversalCRT() const;
   bool getUniversalCRTLibraryPath(std::string ) const;
-  bool getVisualStudioInstallDir(std::string ) const;
-  bool getVisualStudioBinariesFolder(const char *clangProgramPath,
- std::string ) const;
   VersionTuple
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList ) const override;
@@ -1196,7 +1196,11 @@
 
   Tool *buildLinker() const override;
   Tool *buildAssembler() const override;
+
 private:
+  std::string VCToolChainPath;
+  bool IsVS2017OrNewer;
+
   VersionTuple getMSVCVersionFromTriple() const;
   VersionTuple 

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 83318.
hamzasood added a comment.

- Re-implemented the PATH searching behaviour (thanks @amccarth for pointing 
that out)
- Updated the base revision.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10809,16 +10809,10 @@
   const char *Exe,
   const char *ClangProgramPath) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10843,33 +10837,17 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
 const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (MSVC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1155,6 +1155,13 @@
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;
 
+  enum class SubDirectoryType {
+Bin,
+Include,
+Lib,
+  };
+  std::string getSubDirectoryPath(SubDirectoryType Type) const;
+
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
@@ -1163,19 +1170,12 @@
   llvm::opt::ArgStringList ) const override;
 
   void AddCudaIncludeArgs(const llvm::opt::ArgList ,
-  llvm::opt::ArgStringList ) const override;
+  llvm::opt::ArgStringList ) const override;
 
-  bool getWindowsSDKDir(std::string , int ,
-std::string ,
-std::string ) const;
   bool getWindowsSDKLibraryPath(std::string ) const;
   /// \brief Check if Universal CRT should be used if available
-  bool useUniversalCRT(std::string ) const;
-  bool getUniversalCRTSdkDir(std::string , std::string ) const;
+  bool useUniversalCRT() const;
   bool getUniversalCRTLibraryPath(std::string ) const;
-  bool getVisualStudioInstallDir(std::string ) const;
-  bool getVisualStudioBinariesFolder(const char *clangProgramPath,
- std::string ) const;
   VersionTuple
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList ) const override;
@@ -1196,7 +1196,11 @@
 
   Tool *buildLinker() const override;
   Tool *buildAssembler() const override;
+
 private:
+  std::string VCToolChainPath;
+  bool IsVS2017OrNewer;
+
   VersionTuple 

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added a comment.

(I could be missing something because the diff doesn't have much context.)

If I'm reading this right, it looks like it will no longer search the PATH in 
order to find cl.exe.  If that's the case, then I'm mildly worried about this 
change in behavior.  I know I have multiple versions of VS installed, and 
commonly select one just by moving its bin directory to the head of PATH.  The 
old behavior would find that one (which is used for things like defaulting 
-fms-compatibility-version) while the new behavior will find the newest one 
installed on the machine.

That's not necessarily a deal breaker, but it's an important change for Windows 
developers to be aware of.


https://reviews.llvm.org/D28365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 83303.
hamzasood added a comment.

- Fixed an error in retrieving a toolchain path from the registry.
- Updated the base revision.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10809,16 +10809,10 @@
   const char *Exe,
   const char *ClangProgramPath) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10843,33 +10837,17 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
 const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (MSVC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1155,6 +1155,13 @@
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;
 
+  enum class SubDirectoryType {
+Bin,
+Include,
+Lib,
+  };
+  std::string getSubDirectoryPath(SubDirectoryType Type) const;
+
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
@@ -1163,19 +1170,12 @@
   llvm::opt::ArgStringList ) const override;
 
   void AddCudaIncludeArgs(const llvm::opt::ArgList ,
-  llvm::opt::ArgStringList ) const override;
+  llvm::opt::ArgStringList ) const override;
 
-  bool getWindowsSDKDir(std::string , int ,
-std::string ,
-std::string ) const;
   bool getWindowsSDKLibraryPath(std::string ) const;
   /// \brief Check if Universal CRT should be used if available
-  bool useUniversalCRT(std::string ) const;
-  bool getUniversalCRTSdkDir(std::string , std::string ) const;
+  bool useUniversalCRT() const;
   bool getUniversalCRTLibraryPath(std::string ) const;
-  bool getVisualStudioInstallDir(std::string ) const;
-  bool getVisualStudioBinariesFolder(const char *clangProgramPath,
- std::string ) const;
   VersionTuple
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList ) const override;
@@ -1196,7 +1196,11 @@
 
   Tool *buildLinker() const override;
   Tool *buildAssembler() const override;
+
 private:
+  std::string VCToolChainPath;
+  bool IsVS2017OrNewer;
+
   VersionTuple getMSVCVersionFromTriple() const;
   

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood created this revision.
hamzasood added reviewers: rnk, ruiu, hans.
hamzasood added a subscriber: cfe-commits.

The patch updates the MSVC ToolChain for the changes made in Visual Studio 2017 
(https://blogs.msdn.microsoft.com/vcblog/2016/10/07/compiler-tools-layout-in-visual-studio-15/).
Other notable changes:

- Path handling code has been centralised to make potential future changes less 
painful.
- A compiler error is emitted if the driver is unable to locate a usable MSVC 
toolchain. (Previously it'd fail with a cryptic error such as "link.exe is not 
executable")
- Support for the new Setup Config Server 

 API has been added, albeit block commented out with a preprocessor 
conditional. This can probably be re-evaluated when the API is officially 
released (it's currently at the RC stage), but it's left in to make it easy for 
anyone familiar with the API to give it a go with Clang.

Patch by Hamza Sood.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10799,16 +10799,10 @@
   const char *Exe,
   const char *ClangProgramPath) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10833,33 +10827,17 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
 const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (MSVC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1140,6 +1140,13 @@
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;
 
+  enum class SubDirectoryType {
+Bin,
+Include,
+Lib,
+  };
+  std::string getSubDirectoryPath(SubDirectoryType Type) const;
+
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
@@ -1147,17 +1154,10 @@
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
 
-  bool getWindowsSDKDir(std::string , int ,
-std::string ,
-std::string ) const;
   bool getWindowsSDKLibraryPath(std::string ) const;
   /// \brief Check if Universal CRT should be used if available
-  bool