aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
Good catch that this was crashing! LGTM aside from the nit pointed out by
@tbaeder. Can you also add a release note for the fix? Do you need someone to
commit on your behalf? If so, what name and email address would you like used
for patch attribution?
================
Comment at: clang/lib/Parse/ParseDecl.cpp:5340
+ if (!getLangOpts().CPlusPlus)
+ return false;
if (NextToken().is(tok::kw_new) || // ::new
----------------
inclyc wrote:
> Maybe we can make a new `error` diagnostic definition and fire that here?
I don't think we should -- this function is used to query whether something is
a declaration specifier or not; it'd be surprising to issue a diagnostic from
that kind of interface.
================
Comment at: clang/test/Parser/c2x-attributes.c:146
+// Ensure that '::' outside of attributes does not crash and is not treated as
scope
+double n::v; // expected-error {{expected ';' after top level declarator}}
----------------
inclyc wrote:
> Could we improve this diagnostic message?
> ```
> expected ';' after top level declarator}
> ```
That might be possible, but it should happen as a separate patch. That said,
I'm not certain how much improvement there is to be had there, especially in C
mode. It really does look like the user is trying to declare a variable named
`n` and got the wrong kind of colon.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133248/new/
https://reviews.llvm.org/D133248
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
