Lgtm.  I also have a hard time saying which is best, we’re basically trying to help people who have already strayed from the path, so there’s probably no objectively correct answer <br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 17, 2018 at 7:26 AM Nico Weber via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">thakis updated this revision to Diff 155885.<br>
thakis added a comment.<br>
<br>
address comment<br>
<br>
<br>
<a href="https://reviews.llvm.org/D49398" rel="noreferrer" target="_blank">https://reviews.llvm.org/D49398</a><br>
<br>
Files:<br>
  lib/Driver/ToolChains/MSVC.cpp<br>
  lib/Driver/ToolChains/MSVC.h<br>
<br>
<br>
Index: lib/Driver/ToolChains/MSVC.h<br>
===================================================================<br>
--- lib/Driver/ToolChains/MSVC.h<br>
+++ lib/Driver/ToolChains/MSVC.h<br>
@@ -123,6 +123,8 @@<br>
<br>
   void printVerboseInfo(raw_ostream &OS) const override;<br>
<br>
+  bool FoundMSVCInstall() const { return !VCToolChainPath.empty(); }<br>
+<br>
 protected:<br>
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,<br>
                                      llvm::opt::ArgStringList &CC1Args,<br>
Index: lib/Driver/ToolChains/MSVC.cpp<br>
===================================================================<br>
--- lib/Driver/ToolChains/MSVC.cpp<br>
+++ lib/Driver/ToolChains/MSVC.cpp<br>
@@ -469,6 +469,9 @@<br>
     // their own link.exe which may come first.<br>
     linkPath = FindVisualStudioExecutable(TC, "link.exe");<br>
<br>
+    if (!TC.FoundMSVCInstall() && !llvm::sys::fs::can_execute(linkPath))<br>
+      C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found);<br>
+<br>
 #ifdef _WIN32<br>
     // When cross-compiling with VS2017 or newer, link.exe expects to have<br>
     // its containing bin directory at the top of PATH, followed by the<br>
@@ -684,8 +687,6 @@<br>
 }<br>
<br>
 Tool *MSVCToolChain::buildLinker() const {<br>
-  if (VCToolChainPath.empty())<br>
-    getDriver().Diag(clang::diag::warn_drv_msvc_not_found);<br>
   return new tools::visualstudio::Linker(*this);<br>
 }<br>
<br>
<br>
<br>
</blockquote></div>