aaron.ballman added inline comments.
================
Comment at: clang/lib/Parse/ParseStmt.cpp:177-178
+ case tok::kw___attribute:
+ case tok::kw_const:
+ case tok::kw_volatile:
+ case tok::star:
----------------
What about other qualifiers? `_Nullable` and `restrict` and whatnot?
================
Comment at: clang/lib/Parse/ParseStmt.cpp:184-185
+ case tok::ampamp:
+ case tok::kw___declspec:
+ case tok::l_square:
+ return P.getLangOpts().CPlusPlus;
----------------
Why are these pinned to C++? `__declspec` is used in C and shows up in the same
syntactic locations as in C++, and `[` seems like you're looking for
`[[]]`-style attributes (perhaps?) but those also exist in C.
What about parens? e.g.,
```
_BitNit(12) foo; // Oops, meant to type _BitInt(12)
```
================
Comment at: clang/test/AST/ast-dump-invalid.c:8-11
+ unknown_t const *c;
+ // CHECK: VarDecl{{.*}} invalid c 'const int *'
+ unknown_t volatile *d;
+ // CHECK: VarDecl{{.*}} invalid d 'volatile int *'
----------------
I'd also like to see a test where the qualifier comes before the unknown type
name:
```
const unknown_t e;
```
and for typos with multi-keyword types:
```
unsinged long g;
unsigned lnog h;
```
and for situations that are just utter nonsense rather than types:
```
unknown_t[12] i;
unknown_t&&& j;
```
ohhh.... and GNU statement expressions make this a bit interesting because
that's a place where a declaration and an expression get a bit mixed up:
```
({ int val = 0, i = 12; vla * i; }) // Typo: vla should be val, but this is
*not* a declaration, it's the expression determining the value of the statement
expression.
({ int val = 0, i = 12; vla & i; }) // Similar but an interesting case for C++
```
================
Comment at: clang/test/Parser/recovery.c:109
+ unknown_t __attribute__ ((aligned (16))) f; // expected-error {{unknown type
name 'unknown_t'}}
+ int g = unknown_t * a; // expected-error {{use of undeclared identifier
'unknown_t'}}
+}
----------------
There is a behavioral difference here that is plausibly a regression: we used
to get `use of undeclared identifier 'a'` here as well but because we recover
as-if `a` was declared as type `int`, that diagnostic has gone away. I think
that's reasonable as the user was already told about the issue with the
declaration of `a`.
================
Comment at: clang/test/Parser/recovery.c:105
+ unknown_t a; // expected-error {{unknown type name 'unknown_t'}}
+ unknown_t *b; // expected-error {{unknown type name 'unknown_t'}}
+ unknown_t const *c; // expected-error {{unknown type name 'unknown_t'}}
----------------
urazoff wrote:
> sammccall wrote:
> > this diagnostic is worse than the old one (less accurate).
> >
> > (I think it's OK to trade off diagnostics quality if it's better on
> > balance, maybe leave a comment?)
> Just to mention, this is exact behavior of clang for C++.
> this diagnostic is worse than the old one (less accurate).
Less accurate how? It went from `use of undeclared identifier 'unknown_t'` to
`unknown type name 'unknown_t'` and that seems accurate to me given that the
undeclared identifier is written in a type position.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137020/new/
https://reviews.llvm.org/D137020
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
