[patch, clang-cl] Implement /Zc:trigraphs support, disable trigraphs by default

[Nico Weber]

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 <hans at chromium.org> 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 <thakis at chromium.org> wrote:
> > Thanks! All done.
> >
> > On Fri, Dec 19, 2014 at 6:47 PM, Richard Smith <richard at metafoo.co.uk>
> > wrote:
> >>
> >> On Fri, Dec 19, 2014 at 6:08 PM, Nico Weber <thakis at chromium.org>
> wrote:
> >>>
> >>> Forgot to say: Fixes PR21974.
> >>>
> >>> On Fri, Dec 19, 2014 at 6:07 PM, Nico Weber <thakis at chromium.org>
> 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
> >>> cfe-commits at cs.uiuc.edu
> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >>>
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141221/8d0cf4d6/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang-cl-trigraphs.patch
Type: application/octet-stream
Size: 13819 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141221/8d0cf4d6/attachment.obj>