On Tue, Oct 31, 2017 at 12:17 PM, Mukesh Kapoor
<mukesh.kap...@oracle.com> wrote:
> On 10/25/2017 6:44 PM, Mukesh Kapoor wrote:
>>
>> On 10/25/2017 4:20 AM, Nathan Sidwell wrote:
>>>
>>> On 10/25/2017 12:03 AM, Mukesh Kapoor wrote:
>>>
>>>> Thanks for pointing this out. Checking in the front end will be
>>>> difficult because the front end gets tokens after macro expansion. I think
>>>> the difficulty of fixing this bug comes because of the requirement to
>>>> maintain backward compatibility with the option -Wliteral-suffix for
>>>> -std=c++11.
>>>
>>>
>>> IIUC the warning's intent is to catch cases of:
>>> printf ("some format"PRIx64 ..., ...);
>>> where there's no space between the string literals and the PRIx64 macro.
>>> I suspect it's very common for there to be a following string-literal, so
>>> perhaps the preprocessor could detect:
>>>
>>> <string-literal>NON-FN-MACRO<maybe-space><string-literal>
>>>
>>> and warn on that sequence?
>>
>>
>> Yes, this can be done easily and this is also the usage mentioned in the
>> man page. I made this change in the compiler, bootstrapped it and ran the
>> tests. The following two tests fail after the fix:
>>
>> g++.dg/cpp0x/Wliteral-suffix.C
>> g++.dg/cpp0x/warn_cxx0x4.C
>>
>> Both tests have code similar to the following (from Wliteral-suffix.C):
>>
>> #define BAR "bar"
>> #define PLUS_ONE + 1
>>
>>   char c = '3'PLUS_ONE;   // { dg-warning "invalid suffix on literal" }
>>   char s[] = "foo"BAR;    // { dg-warning "invalid suffix on literal" }
>>
>> Other compilers don't accept this code. Maybe I should just modify these
>> tests to have error messages instead of warnings and submit my revised fix?
>
>
> Actually, according to the man page for -Wliteral-suffix, only macro names
> that don't start with an underscore should be considered when issuing a
> warning:
>
>        -Wliteral-suffix (C++ and Objective-C++ only)
>            Warn when a string or character literal is followed by a
> ud-suffix
>            which does not begin with an underscore...
>
> So the fix is simply to check if the macro name in is_macro() starts with an
> underscore. The function is_macro() is called only at three places. At two
> places it's used to check for the warning related to -Wliteral-suffix and
> the check for underscore should be made for these two cases; at one place it
> is used to check for the warning related to -Wc++11-compat and there is no
> need to check for underscore for this case.
>
> The fix is simply to pass a bool flag as an additional argument to
> is_macro() to decide whether the macro name starts with an underscore or
> not. I have tested the attached patch on x86_64-linux. Thanks.

Rather than add a mysterious parameter to is_macro, how about checking
*cur != '_' before we call it?

Jason

Reply via email to