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