From: Jason Merrill <jason@redhat.com>
To: David Malcolm <dmalcolm@redhat.com>
Cc: gcc-patches List <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)
Date: Wed, 07 Feb 2018 18:22:00 -0000 [thread overview]
Message-ID: <CADzB+2mz2uMh3xbEnamXLZ4yKWmhFiHz4z=i8+oJeaSUXg26VQ@mail.gmail.com> (raw)
In-Reply-To: <1518027125.26503.71.camel@redhat.com>
On Wed, Feb 7, 2018 at 1:12 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Wed, 2018-02-07 at 12:21 -0500, Jason Merrill wrote:
>> On Fri, Jan 26, 2018 at 1:12 PM, David Malcolm <dmalcolm@redhat.com>
>> wrote:
>> > On Mon, 2017-12-11 at 17:24 -0500, Jason Merrill wrote:
>> > > On Wed, Nov 22, 2017 at 10:36 AM, David Malcolm <dmalcolm@redhat.
>> > > com>
>> > > wrote:
>> >
>> > Original post:
>> > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02048.html
>> >
>> > > > PR c++/81610 and PR c++/80567 report problems where the C++
>> > > > frontend
>> > > > suggested "if", "for" and "else" as corrections for misspelled
>> > > > variable
>> > > > names.
>> >
>> > I've now marked these PRs as regressions: the nonsensical
>> > suggestions
>> > are only offered by trunk, not by gcc 7 and earlier.
>> >
>> > > Hmm, what about cases where people are actually misspelling
>> > > keywords?
>> > > Don't we want to handle that?
>> > >
>> > > fi (true) { }
>> > > retrun 42;
>> >
>> > I'd prefer not to.
>> >
>> > gcc 7 and earlier don't attempt to correct the spelling of the "fi"
>> > and
>> > "retrun" above.
>> >
>> > trunk currently does offer "return" as a suggestion, but it was by
>> > accident, and I'm wary of attempting to support these corrections:
>> > is
>> > "fi" meant to be an "if", or a function call that's missing its
>> > decl,
>> > or a name lookup issue? ...etc
>> >
>> > > In the PRs you mention, the actual identifiers are 1) missing
>> > > includes, which we should check first, and 2) pretty far from the
>> > > suggested keywords.
>> >
>> > The C++ FE is missing a suggestion about which #include to use for
>> > "memset", but I'd prefer to treat that as a follow-up patch (and
>> > probably for next stage 1).
>> >
>> > In the meantime, is this patch OK for trunk? (as a regression fix)
>>
>> Yes.
>
> Thanks; committed (r257456).
>
> FWIW, I've filed PR c++/84269 so I remember to fix the missing
> suggestion for "memset" (in gcc 9 stage1).
Did you have a reaction to my comment about the suggested keyword
being pretty far from the actual identifier? Do we want to lower the
cutoff for suggestions at all?
Jason
next prev parent reply other threads:[~2018-02-07 18:22 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-22 16:35 David Malcolm
2017-12-11 16:10 ` PING: " David Malcolm
2017-12-11 22:25 ` Jason Merrill
2018-01-26 20:15 ` David Malcolm
2018-02-02 21:18 ` [PING] " David Malcolm
2018-02-07 17:21 ` Jason Merrill
2018-02-07 18:12 ` David Malcolm
2018-02-07 18:22 ` Jason Merrill [this message]
2018-02-07 18:28 ` David Malcolm
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+2mz2uMh3xbEnamXLZ4yKWmhFiHz4z=i8+oJeaSUXg26VQ@mail.gmail.com' \
--to=jason@redhat.com \
--cc=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.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).