Re: [PATCH] C++: avoid most reserved words as misspelling suggestions (PR c++/81610 and PR c++/80567)

[David Malcolm <dmalcolm@redhat.com>]

On Wed, 2018-02-07 at 13:22 -0500, Jason Merrill wrote:
> 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.c
> > > om>
> > > 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@red
> > > > > hat.
> > > > > 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?

I've played around with tweaking how the cutoff works, [1] in response
to e.g. PR c/82967 ('"did you mean" suggestions are way too
suggestive'), but I've not yet come up with a version I prefer to the
current implementation.