[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<const char *>`. We can get the right lifetime with `Args.MakeArgString`.


https://reviews.llvm.org/D28365