On Mon, Apr 11, 2016 at 7:16 PM, Nico Weber <tha...@chromium.org> wrote: > r260990 exposed -isystem in clang-cl. -isystem adds a directory to the front > of the system include search path. The idea was to use this to point to a > hermetic msvc install, but as it turns out this doesn't work: -isystem then > adds the hermetic headers in front of clang's builtin headers, and clang's > headers that are supposed to wrap msvc headers (say, stdarg.h) aren't picked > up at all anymore. > > So revert that, and instead expose -imsvc which works as if the passed > directory was part of %INCLUDE%: The header is treated as a system header, > but it is searched after clang's lib/Header headers.
Sounds great. I wonder if this is useful beyond clang-cl and should have a more general name, but I can't think of anything, so let's go with this. > Also expose -nostdlibinc so that clang-cl can be told to not look for system > headers in the usual places. Can you land this in a separate commit? > With this change, it's possible to use -imsvc to point clang-cl at a > hermetic MSVC installation and not make it look at the headers of a possibly > installed msvc. Fixes PR26751. > > I'd upload this to phab, but it's currently down. Old-school review below: > Index: include/clang/Driver/CLCompatOptions.td > =================================================================== > --- include/clang/Driver/CLCompatOptions.td (revision 265325) > +++ include/clang/Driver/CLCompatOptions.td (working copy) > @@ -210,6 +210,8 @@ > HelpText<"Enable exception handling">; > def _SLASH_GX_ : CLFlag<"GX-">, > HelpText<"Enable exception handling">; > +def _SLASH_imsvc : CLJoinedOrSeparate<"imsvc">, > + HelpText<"Add directory to system include search path, as if part of > %INCLUDE%">, MetaVarName<"<dir>">; Nit: I'd have gone with a line break before MetaVarName to pretend we're at least trying to keep the lines short. > Index: include/clang/Driver/Options.td > =================================================================== > --- include/clang/Driver/Options.td (revision 265325) > +++ include/clang/Driver/Options.td (working copy) > @@ -1246,7 +1246,7 @@ > def isysroot : JoinedOrSeparate<["-"], "isysroot">, Group<clang_i_Group>, > Flags<[CC1Option]>, > HelpText<"Set the system root directory (usually /)">, > MetaVarName<"<dir>">; > def isystem : JoinedOrSeparate<["-"], "isystem">, Group<clang_i_Group>, > - Flags<[CC1Option, CoreOption]>, > + Flags<[CC1Option]>, > HelpText<"Add directory to SYSTEM include search path">, > MetaVarName<"<directory>">; > def iwithprefixbefore : JoinedOrSeparate<["-"], "iwithprefixbefore">, > Group<clang_i_Group>, > HelpText<"Set directory to include search path with prefix">, > MetaVarName<"<dir>">, > @@ -1673,7 +1673,7 @@ > def noseglinkedit : Flag<["-"], "noseglinkedit">; > def nostartfiles : Flag<["-"], "nostartfiles">; > def nostdinc : Flag<["-"], "nostdinc">; > -def nostdlibinc : Flag<["-"], "nostdlibinc">; > +def nostdlibinc : Flag<["-"], "nostdlibinc">, Flags<[CoreOption]>; Commit this one separately. > Index: lib/Driver/MSVCToolChain.cpp > =================================================================== > --- lib/Driver/MSVCToolChain.cpp (revision 265325) > +++ lib/Driver/MSVCToolChain.cpp (working copy) > @@ -527,6 +527,10 @@ > "include"); > } > > + // Add %INCLUDE%-like directories from the -systemI flag. > + for (const auto &path : > DriverArgs.getAllArgValues(options::OPT__SLASH_imsvc)) > + addSystemInclude(DriverArgs, CC1Args, path); Update flag name in the comment. Spelling the loop variable Path would be more LLVM-style. > Index: test/Driver/cl-options.c > =================================================================== > --- test/Driver/cl-options.c (revision 265325) > +++ test/Driver/cl-options.c (working copy) > @@ -82,6 +82,10 @@ > // RUN: %clang_cl /I myincludedir -### -- %s 2>&1 | FileCheck > -check-prefix=SLASH_I %s > // SLASH_I: "-I" "myincludedir" > > +// RUN: %clang_cl /imsvcmyincludedir -### -- %s 2>&1 | FileCheck > -check-prefix=SLASH_imsvc %s > +// RUN: %clang_cl /imsvc myincludedir -### -- %s 2>&1 | FileCheck > -check-prefix=SLASH_imsvc %s > +// SLASH_imsvc: "-internal-isystem" "myincludedir" Would it be possible to check that this flag comes after the -internal-isystem for the resource headers? _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
