hans added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:1255 + } + } + ---------------- thakis wrote: > `/external:env` should happen after winsysroot I think. sysroots try to make > builds hermetic and env vars defeat that. > > I.e. this flag is more like the env var reads below than imsvc above – it > reads the env mostly, instead of being a flag mostly. Hmm. I guess it depends on how people will use this flag, which isn't entirely clear. The way I think about it, users might use this to point to third-party libraries besides the system headers. For that reason it's different from the env var reads below, which are about finding the system headers. In particular, I don't think /nostdlibinc (/X) should disable these flags, which is why I put it before that check. I don't think people would combine this with /winsysroot, but if they want to, I'm not sure we should prevent it. ================ Comment at: clang/test/Driver/cl-include.c:10 -// RUN: env INCLUDE=/my/system/inc %clang_cl -### -- %s 2>&1 | FileCheck %s --check-prefix=STDINC +// RUN: env INCLUDE=/my/system/inc env EXTERNAL_INCLUDE=/my/system/inc2 %clang_cl -### -- %s 2>&1 | FileCheck %s --check-prefix=STDINC // STDINC: "-internal-isystem" "/my/system/inc" ---------------- thakis wrote: > Should there be tests for the interaction with `/X`, `/winsysroot:`? Yes, adding that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104387/new/ https://reviews.llvm.org/D104387 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
