That's much nicer, thanks. I couldn't figure out how to alias a clang-cl option to a cc1 option, so I added 4 lines to explicitly translate OPT__SLASH_Zc_trigraphs to -ftrigraphs, but it's still much simpler.
I also used the same technique to map /Zc:strictStrings to -Werror=c++11-compat-deprecated-writable-strings while here. (Disabling trigraphs in -fms-compatibility mode for the gcc driver makes sense to me too, but I think that should happen in a different patch.) On Sun, Dec 21, 2014 at 4:36 AM, Hans Wennborg <[email protected]> wrote: > I would have broken out /Zc:trigraphs[-] into two separate CLFlag > options instead. > > That way we don't need parser code for it, the flags can have > individual help texts, and they can just be aliases to the cc1 flags. > > Also, instead of passing -fno-trigraphs each time, maybe we should > change the default in CompilerInvocation based on the language > options. It seems we currently disable them in gnu mode for example. > Maybe we should do the same if -fms-extensions or -fms-compatibility > if enabled? > > On Sat, Dec 20, 2014 at 11:16 AM, Nico Weber <[email protected]> wrote: > > Thanks! All done. > > > > On Fri, Dec 19, 2014 at 6:47 PM, Richard Smith <[email protected]> > > wrote: > >> > >> On Fri, Dec 19, 2014 at 6:08 PM, Nico Weber <[email protected]> > wrote: > >>> > >>> Forgot to say: Fixes PR21974. > >>> > >>> On Fri, Dec 19, 2014 at 6:07 PM, Nico Weber <[email protected]> > wrote: > >>>> > >>>> Hi, > >>>> > >>>> the attached patch disables trigraphs by default in clang-cl mode. To > do > >>>> so, I'm renaming the cc1 trigraph flag to -ftrigraphs, and I'm adding > a > >>>> -fno-trigraphs flag. (This requires updating a handful of cc1 tests, > and > >>>> translating -trigraphs to -ftrigraphs in the driver.) > >>>> > >>>> The driver grows parsing logic for /Zc:, everything other than > trigraphs > >>>> is ignored for now. (We probably want to implement at least > /Zc:inline at > >>>> some point, which is why I added parsing code for this instead of > always > >>>> sending -fno-trigraphs to cc1 in clang-cl mode.) > >> > >> > >> -def trigraphs : Flag<["-", "--"], "trigraphs">, Flags<[CC1Option]>, > >> +def trigraphs : Flag<["-", "--"], "trigraphs">, Flags<[DriverOption]>, > >> > >> I don't think DriverOption is right here; that means we don't forward > the > >> flag to a gcc invocation. Now, we never invoke GCC for C-like languages > any > >> more, but if we did, this would be wrong (and for all I know, it's > wrong for > >> Fortran...). You should be able to just drop the Flags. > >> > >> > >> if (Arg *A = Args.getLastArg(options::OPT_std_EQ, > options::OPT_ansi, > >> - options::OPT_trigraphs)) > >> - if (A != Std) > >> + options::OPT_trigraphs)) { > >> + if (A->getOption().matches(options::OPT_trigraphs)) > >> + CmdArgs.push_back("-ftrigraphs"); > >> + else if (A != Std) > >> A->render(Args, CmdArgs); > >> > >> Std here is Args.getLastArg(options::OPT_std_EQ, options::OPT_ansi), so > >> your 'else if' clause is unreachable. Something like this would be > simpler > >> and more obvious: > >> > >> // If -trigraphs appears after the language standard flag, honor it. > >> if (Args.getLastArg(options::OPT_std_EQ, options::OPT_ansi, > >> options::OPT_trigraphs) != Std) > >> CmdArgs.push_back("-ftrigraphs"); > >> > >> You could even sink this below the 'if (Std)' block to avoid duplicating > >> it between the cases where we do and don't have a Std flag. > >> > >>>> Ok? > >>>> > >>>> Nico > >>> > >>> > >>> > >>> _______________________________________________ > >>> cfe-commits mailing list > >>> [email protected] > >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >>> > > > > > > _______________________________________________ > > cfe-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > >
clang-cl-trigraphs.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
