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 > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
