From: Jason Merrill <jason@redhat.com>
To: Mukesh Kapoor <mukesh.kapoor@oracle.com>
Cc: Nathan Sidwell <nathan@acm.org>,
gcc-patches List <gcc-patches@gcc.gnu.org>
Subject: Re: [C++ Patch] PR 80955 (Macros expanded in definition of user-defined literals)
Date: Thu, 02 Nov 2017 14:42:00 -0000 [thread overview]
Message-ID: <CADzB+2=5ArhKna0KVvqV6Mc8UDb0kZXexc5YyYYCKm0Vpbk_qQ@mail.gmail.com> (raw)
In-Reply-To: <f85e7d56-05a8-1f8f-1af6-18862074a2b3@oracle.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
next prev parent reply other threads:[~2017-11-02 14:42 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-20 17:03 Mukesh Kapoor
2017-10-20 18:00 ` Nathan Sidwell
2017-10-20 18:19 ` Mukesh Kapoor
2017-10-20 20:56 ` Mukesh Kapoor
2017-10-24 14:07 ` Jason Merrill
2017-10-25 5:48 ` Mukesh Kapoor
2017-10-25 11:26 ` Nathan Sidwell
2017-10-25 16:13 ` Jason Merrill
2017-10-25 16:14 ` Mukesh Kapoor
2017-10-26 3:30 ` Mukesh Kapoor
2017-10-31 16:26 ` Mukesh Kapoor
2017-11-01 20:02 ` Jason Merrill
2017-11-01 20:44 ` Mukesh Kapoor
2017-11-02 14:42 ` Jason Merrill [this message]
2017-11-03 14:31 ` Paolo Carlini
2017-11-04 22:38 ` Mukesh Kapoor
2017-11-06 10:30 ` Paolo Carlini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CADzB+2=5ArhKna0KVvqV6Mc8UDb0kZXexc5YyYYCKm0Vpbk_qQ@mail.gmail.com' \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=mukesh.kapoor@oracle.com \
--cc=nathan@acm.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).