aganea added subscribers: amccarth, dblaikie.
aganea added a comment.
In D85998#2231001 <https://reviews.llvm.org/D85998#2231001>, @zahen wrote:
> The build system strives to be deterministic
When you say build system, you mean MSBuild? Other? For example, Fastbuild
purposely calls `CreateProcess` with an empty environment block (not null) to
avoid determinism issues. Is that your case? We are also using `-nostdinc` to
avoid any compiler-generated include paths, even in cases where you need to
repro something in a VS cmd shell.
The patch looks good otherwise, modulo the comments. + @amccarth @dblaikie for
more opinions.
================
Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:74
+ if (Arg *A = Args.getLastArg(options::OPT__SLASH_vctoolsdir)) {
+ Path = A->getValue();
+ VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
----------------
zahen wrote:
> Deliberately not validating the input. The primary motivation is to prevent
> unnecessary file and registry access.
I would add what you said above as a comment. It is interesting for future
readers of this code, and avoids digging the history for intent.
================
Comment at: clang/test/Driver/cl-options.c:686
+// vctoolsdir is handled by the driver; just check that we don't error. Pass
-c because fakedir isn't a real toolchain path
+// RUN: %clang_cl -c -vctoolsdir fakedir -- %s 2>&1
+
----------------
Check that we're not detecting a local installation, and that we fallback to
the default triple 19.11, ie.
```
// RUN: %clang_cl ... -vctoolsdir "" -### | FileCheck %s --check-prefix
VCTOOLSDIR
// VCTOOLSDIR: "-cc1" "-triple" "x86_64-pc-windows-msvc19.11.0"
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85998/new/
https://reviews.llvm.org/D85998
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
