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

[Hans Wennborg]

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.

> 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
>> >
>
>