[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff created this revision. urazoff added reviewers: sammccall, hokein, adamcz. Herald added a project: All. urazoff requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. With this patch, declarations containing unknown types are parsed li

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff added a comment. https://github.com/llvm/llvm-project/issues/58355 corresponding github issue with current clang behavior. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137020/new/ https://reviews.llvm.org/D137020

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

shafik added subscribers: aaron.ballman, shafik. shafik added a comment. CC @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137020/new/ https://reviews.llvm.org/D137020 ___ cfe-commits maili

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

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 +boo

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:5392-5394 + case tok::amp: + case tok::ampamp: +return getLangOpts().CPlusPlus; aaron.ballman wrote: > Looking for `*`, `&`, and `&&` will help catch some cases... but it's not > real

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

sammccall added a comment. I admire the goals of this patch, but I think it needs some explanation "why we expect this to be roughly correct/safe", rather than just trying to think of cases that might go wrong and patch them. The test coverage for incorrect code is not good, I'd expect to need

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff added inline comments. 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; sammccall wrote: > urazoff wrote: > > aaron

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff updated this revision to Diff 475370. urazoff added a comment. Added test to show the advantage in AST dump. Missing keywords are added in 'IsUnknownTypedefName', the function is static now. 'DisambiguatingWithExpression' check is added to narrow down the effect of the changes. CHANGES

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff 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: aaron.ballman wrote: > What about other qualifiers? `_Nullable` and `restrict` and wh

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

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: urazoff wrote: > aaron.ballman wrote: > > What about other qualifiers? `_Nullab

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff added inline comments. Comment at: clang/lib/Parse/ParseStmt.cpp:184-185 + case tok::ampamp: + case tok::kw___declspec: + case tok::l_square: +return P.getLangOpts().CPlusPlus; aaron.ballman wrote: > Why are these pinned to C++? `__declspec` is use

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff added a comment. @sammccall @aaron.ballman I am thinking about another solution with tentative parsing as implemented for C++ in `Parser::isCXXSimpleDeclaration` (which is eventually called from `isDeclarationStatement()`). This approach works well for C++. So I want to update this patc

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff updated this revision to Diff 476766. urazoff added a comment. - Added test for AST dump of invalid C code - Added testcase for diagnostics - Some minor fixes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137020/new/ https://reviews.llvm.org/D137020 Files: clang/lib/Parse/Par

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

sammccall added a comment. Not sure this is ready for review again, ignore me if not... I'm still not sure why this is correct in principle. Without that, if someone finds a misparse 6 months from now I don't know how we determine where to fix it. For example, this path is called from `Parser

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

aaron.ballman added a comment. In D137020#3940523 , @sammccall wrote: > Not sure this is ready for review again, ignore me if not... > > I'm still not sure why this is correct in principle. Without that, if someone > finds a misparse 6 months from now I

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff added inline comments. 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 'unkno

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

urazoff updated this revision to Diff 478667. urazoff added a comment. Reasoning about invalid code is made now in specific parsing path not in general decision-making method. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137020/new/ https://reviews.llvm.org/D137020 Files: clang/lib

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

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?

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

sammccall added inline comments. 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 'unk

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

aaron.ballman added inline comments. 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