hokein marked 2 inline comments as done.
hokein added inline comments.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:6872
+ // Recovery if a keyword is used as an identifier.
+ if (Tok.getIdentifierInfo() &&
----------------
sammccall wrote:
> As I understand it, we're trying to catch keywords in place of the identifier
> in a declarator, and in particular the common case where:
> - the identifier comes at the end of the declarator
> - if the identifier is dropped, the declarator is valid but anonymous (no
> identifier)
> - so declarator parsing succeeds, and then we have a trailing keyword, which
> is never valid in a param list (need , or ... or `)` )
>
> And we can't handle this in ParseDeclarator because in general keywords may
> be allowed to follow the declarator. (And in some cases there'd be better
> recovery like inserting punctuation).
> Our recovery is just treating this as an anonymous parameter, and fortunately
> we've already almost done that.
>
> So all of this would be good to capture in the comment :-)
>
> We should restrict to these cases as much as possible: the type should be
> present, and the identifier should be null[1].
>
> I don't know how confident we can be all the time that our interpretation
> (the keyword was meant to be a variable) is right, and the diagnostic might
> be confusing in that case.
> e.g. `void foo(int const)` will trigger this diagnostic, and the user may
> intend anonymous `void foo(const int)`. `using keyword const as an identifier
> is not permitted` may be less clear than e.g. the more specific `invalid
> parameter name: 'const' is a keyword`.
>
> (Note that because of the [1] case, as the patch stands `void foo(int x
> const)` will trigger `using keyword const as an identifier...`, which is bad!)
yes, exactly, added comments. I first tried to handle it in ParseDeclarator,
but failed with that.
And `void foo(int const)` would not trigger the diagnostic -- this is because
it is a valid declarator, the identifier is dropped, case[2].
by restricting the case (only emit the diagnostic when the type is known, and
identifier is null), `void foo(int x const)` will not trigger the diagnostic --
the type and identifier are known.
================
Comment at: clang/test/Parser/cxx-keyword-identifiers.cpp:12
+ // FIXME: we shoud improve the dianostics for the following cases.
+ int case; // expected-error {{expected unqualified-id}}
+ struct X {
----------------
sammccall wrote:
> this seems nice but harder, I think the grammar is more permissive here. I
> guess we'd be able to whitelist the keywords that can legitimately follow?
yeah, I think at least we could append a message to the
`expected-unqualified-id` when the next Tok is a keyword.
================
Comment at: clang/test/Parser/cxx-keyword-identifiers.cpp:17
+int foo6(int case __attribute((weak))); // expected-error {{invalid
parameter}} \
+ // expected-error {{expected ')'}}
expected-note {{to match this '('}}
+
----------------
unfortunately, the error recovery doesn't handle well for this case, we emit a
spurious diagnostic, but I think it is a fair tradeoff.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77633/new/
https://reviews.llvm.org/D77633
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
