All done. phab's back, but since we started with a patch attachment, let's keep it that way.
On Tue, Apr 12, 2016 at 12:12 PM, Hans Wennborg <h...@chromium.org> wrote: > 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? >
clang-cl-imsvc.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
