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

Hans Wennborg hans at chromium.org
Sun Dec 21 04:36:25 PST 2014


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
>



More information about the cfe-commits mailing list