On Wed, Sep 22, 2021 at 7:05 PM Michel Morin <mimomo...@gmail.com> wrote:
On Thu, Sep 23, 2021 at 5:09 AM Jason Merrill <ja...@redhat.com> wrote:
>
> On 9/21/21 20:53, Michel Morin wrote:
> > On Tue, Sep 21, 2021 at 5:24 AM Jason Merrill
<ja...@redhat.com> wrote:
> >>
> >> On 9/17/21 13:31, Michel Morin wrote:
> >>> On Fri, Sep 17, 2021 at 3:23 AM Jason Merrill
<ja...@redhat.com> wrote:
> >>>>
> >>>> On 9/16/21 11:50, Michel Morin wrote:
> >>>>> On Thu, Sep 16, 2021 at 5:44 AM Jason Merrill
<ja...@redhat.com> wrote:
> >>>>>>
> >>>>>> On 9/14/21 04:29, Michel Morin via Gcc-patches wrote:
> >>>>>>> On Tue, Sep 14, 2021 at 7:14 AM David Malcolm
<dmalc...@redhat.com> wrote:
> >>>>>>>>
> >>>>>>>> On Tue, 2021-09-14 at 03:35 +0900, Michel Morin via
Gcc-patches wrote:
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> PR77565 reports that, with the code `typdef int Int;`,
GCC emits
> >>>>>>>>> "did you mean 'typeof'?" instead of "did you mean
'typedef'?".
> >>>>>>>>>
> >>>>>>>>> This happens because the typo corrector determines that
`typeof` is a
> >>>>>>>>> candidate for suggestion (through
> >>>>>>>>> `cp_keyword_starts_decl_specifier_p`),
> >>>>>>>>> but `typedef` is not.
> >>>>>>>>>
> >>>>>>>>> This patch fixes the issue by adding `typedef` as a
candidate. The
> >>>>>>>>> patch
> >>>>>>>>> additionally adds the `inline` specifier and
cv-specifiers as a
> >>>>>>>>> candidate.
> >>>>>>>>> Here is a patch (tests `make check-gcc` pass on darwin):
> >>>>>>>>
> >>>>>>>> Thanks for this patch (and for reporting the bug in the
first place).
> >>>>>>>>
> >>>>>>>> I notice that, as well as being used for fix-it hints by
> >>>>>>>> lookup_name_fuzzy (indirectly via suggest_rid_p),
> >>>>>>>> cp_keyword_starts_decl_specifier_p is also used by
> >>>>>>>> cp_lexer_next_token_is_decl_specifier_keyword, which is
used by
> >>>>>>>> cp_parser_lambda_declarator_opt and
cp_parser_constructor_declarator_p.
> >>>>>>>
> >>>>>>> Ah, you're right! Thank you for pointing this out.
> >>>>>>> I failed to grep those functions somehow.
> >>>>>>>
> >>>>>>> One thing that confuses me is that
cp_keyword_starts_decl_specifier_p
> >>>>>>> misses many keywords that can start decl-specifiers (e.g.
> >>>>>>> typedef/inline/cv-qual and friend/explicit/virtual).
> >>>>>>> So let's wait C++ frontend maintainers ;)
> >>>>>>
> >>>>>> That is strange. Let's add all the rest of them as well.
> >>>>>
> >>>>> Done. Thanks for your help!
> >>>>>
> >>>>> One more thing — cp_keyword_starts_decl_specifier_p
includes RID_ATTRIBUTE
> >>>>> (from the beginning; see https://gcc.gnu.org/PR28261 ), but
attributes are
> >>>>> not decl-specifiers. Would it be reasonable to remove this?
> >>>>
> >>>> It looks like the place that PR28261 used
> >>>> cp_lexer_next_token_is_decl_specifier_keyword specifically
exempts
> >>>> attributes:
> >>>>
> >>>>> &&
(!cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)
> >>>>> /* GNU attributes can actually appear both
at the start of
> >>>>> a parameter and parenthesized declarator.
> >>>>> S (__attribute__((unused)) int);
> >>>>> is a constructor, but
> >>>>> S (__attribute__((unused)) foo) (int);
> >>>>> is a function declaration. */
> >>>>> || (cp_parser_allow_gnu_extensions_p (parser)
> >>>>> &&
cp_next_tokens_can_be_gnu_attribute_p (parser)))
> >>>>
> >>>> So yes, let's remove RID_ATTRIBUTE and the || clause there.
I'd keep
> >>>> the comment, but move it to go with the test for C++11
attributes below.
> >>>
> >>> Done. No regressions introduced.
> >>>
> >>>>> One more thing — cp_keyword_starts_decl_specifier_p
includes RID_ATTRIBUTE
> >>>>> (from the beginning; see https://gcc.gnu.org/PR28261 ), but
attributes are
> >>>>> not decl-specifiers.
> >>>
> >>> Oh, this is wrong. I thought that, since C++11 attributes are
not a
> >>> decl-specifier, neither are GNU attributes. But the comment
just before
> >>> cp_parser_decl_specifier_seq says that GNU attributes are
considered as a
> >>> decl-specifier. So I'm not confident about the removal of
RID_ATTRIBUTE in
> >>> cp_keyword_starts_decl_specifier_p...
> >>
> >> GNU attributes can appear in lots of places, and the only two
callers of
> >> cp_parser_next_token_is_decl_specifier_keyword don't want to treat
> >> attributes accordingly.
> >
> > Makes sense.
> >
> >> Let's go with both your patches, and also
> >> remove the consequently-unnecessary attributes check in
> >> cp_parser_lambda_declarator_opt:
> >>
> >>> if (cp_lexer_next_token_is_decl_specifier_keyword
(parser->lexer)
> >>> && !cp_next_tokens_can_be_gnu_attribute_p (parser))
> >>
> >> OK with that change.
> >
> > Updated and rebased the patch. No regressions on
x86_64-apple-darwin.
> >
> > Thank you for your help!
>
> Looks good, thanks. You can push the patches yourself, right?
This is my first patch contribution to GCC, and I don't have write
access.
So it'd be great if someone pushes the patches.
Ah! Sorry, I was confusing you with Mikael Morin <mikael.mo...@sfr.fr>,
who does have write access.
I assume these patches are small enough that copyright assignment or
DCO certification are not needed. (If needed, I'll prepare one.)
Agreed.
I applied the patches, adjusting the name in the commit to match the
name on your email (rather than "morinmorin").
Thanks!
Jason
