On Wed, Nov 1, 2017 at 4:45 PM, Mukesh Kapoor <mukesh.kap...@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.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? > > This is a good suggestion. I have attached the revised patch. Thanks.
OK, thanks! Jason