aaron.ballman added reviewers: clang-language-wg, aaron.ballman.
aaron.ballman added a comment.
Thank you for the changes! One thing you should add is a release note so users
know there's been a diagnostic improvement.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:5384-5385
+bool Parser::IsUnknownTypedefName() {
+ switch (NextToken().getKind()) {
+ default:
----------------
This is a pretty unsafe function to call from arbitrary parsing contexts. We
should be asserting that the parser is in a reasonable state to even ask this
question; alternatively, we could make this a static function that accepts a
`Parser&` object.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:5388
+ return false;
+ case tok::kw___attribute:
+ case tok::star:
----------------
Why only `__attribute__` and not `__declspec`? `[[]]` attributes as well?
================
Comment at: clang/lib/Parse/ParseDecl.cpp:5392-5394
+ case tok::amp:
+ case tok::ampamp:
+ return getLangOpts().CPlusPlus;
----------------
Looking for `*`, `&`, and `&&` will help catch some cases... but it's not
really going to help for situations like `unknown const *` or other
qualifiers...
================
Comment at: clang/lib/Parse/ParseDecl.cpp:5425
+ // node in AST for such cases which is good for AST readers.
+ if (IsUnknownTypedefName() && !getLangOpts().ObjC)
+ return true;
----------------
Why is ObjC exempted from this?
I need to think about this a whole lot more. On the one hand, I like the
changes happening in the test cases; those look like good changes to me. But on
the other hand, this function is called in a fair number of places to make
parsing decisions and I'm not convinced that speculative answers are the best
way to go from this interface. I need to see if I can find any situations where
this is called and we'd get worse parsing behavior (or incorrect parsing
behavior!).
================
Comment at: clang/test/SemaOpenCL/invalid-pipes-cl1.2.cl:29-34
reserve_id_t r;
#if defined(__OPENCL_C_VERSION__)
-// expected-error@-2 {{use of undeclared identifier 'reserve_id_t'}}
+// expected-error@-2 {{unknown type name 'reserve_id_t'}}
#else
// expected-error@-4 {{unknown type name 'reserve_id_t'}}
#endif
----------------
Repository:
rG LLVM Github Monorepo
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
