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

[Richard Smith]

On Mon, Dec 22, 2014 at 8:40 AM, Nico Weber <thakis at chromium.org> wrote:

> On Mon, Dec 22, 2014 at 1:38 AM, Hans Wennborg <hans at chromium.org> wrote:
>
>> On Sun, Dec 21, 2014 at 5:24 PM, Nico Weber <thakis at chromium.org> wrote:
>> > 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.
>>
>> Hmm, that's a little annoying, but I guess it makes sense.. the
>> clang-cl flags are handled as Driver flags so aliasing to a CC1
>> doesn't work.
>>
>> Perhaps we should just stick to the existing -trigraphs and add a
>> -no-trigraphs driver flag? lgtm either way I guess.
>>
>
> Richard, are you okay with adding a -no-trigraphs gcc-driver-level flag?
> gcc doesn't have this, but several people on the internet are asking for
> this feature (I searched for `gcc "no-trigraphs"`). I'd like a way to build
> with -std=c++11 (i.e. not gnu++11) but still not enable trigraphs myself,
> too.
>

I'd prefer to add -ftrigraphs and -fno-trigraphs to the driver, and make
-trigraphs an alias for -ftrigraphs. Other than that, I think this is a
good idea.


> > I also used the same technique to map /Zc:strictStrings to
>> > -Werror=c++11-compat-deprecated-writable-strings while here.
>>
>> Nice.
>>
>> > (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.)
>>
>> > Property changes on: test/Driver/cl-zc.cpp
>> > ___________________________________________________________________
>> > Added: svn:eol-style
>> > ## -0,0 +1 ##
>> > +LF
>>
>> Is this intentional?
>>
>>
>> > 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/20141223/a3b3bd92/attachment.html>