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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
