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

[Mukesh Kapoor <mukesh dot kapoor at oracle dot com>]

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.

Mukesh

Attachment: CL_80955
Description: Text document

Attachment: patch_80955
Description: Text document