public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: David Malcolm <dmalcolm@redhat.com>
To: Jason Merrill <jason@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:12:00 -0000	[thread overview]
Message-ID: <1518027125.26503.71.camel@redhat.com> (raw)
In-Reply-To: <CADzB+2nNWvYJkFiZvFsmB35h4eq=q7fvaBFz6=iRypDF4nmDXg@mail.gmail.com>

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).

Dave

  reply	other threads:[~2018-02-07 18:12 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 [this message]
2018-02-07 18:22         ` Jason Merrill
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=1518027125.26503.71.camel@redhat.com \
    --to=dmalcolm@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /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).