[patch, clang-cl] Implement /Zc:trigraphs support, disable trigraphs by default
Nico Weber
thakis at chromium.org
Tue Dec 23 14:57:49 PST 2014
Thanks, landed in that form in r224790 / r224791.
On Tue, Dec 23, 2014 at 1:16 PM, Richard Smith <richard at metafoo.co.uk>
wrote:
> 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/68eb1c68/attachment.html>
More information about the cfe-commits
mailing list