Re: [C++ Patch] PR 80955 (Macros expanded in definition of user-defined literals)

[Jason Merrill <jason at redhat dot com>]

On Wed, Nov 1, 2017 at 4:45 PM, Mukesh Kapoor <mukesh.kapoor@oracle.com> wrote:
> On 11/1/2017 1:02 PM, Jason Merrill wrote:
>>
>> On Tue, Oct 31, 2017 at 12:17 PM, Mukesh Kapoor
>> <mukesh.kapoor@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?
>
> This is a good suggestion. I have attached the revised patch. Thanks.

OK, thanks!

Jason