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

[Dilshod Urazov via Phabricator via cfe-commits]

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 really going to help for situations like `unknown const *` or other qualifiers...
Actually, this `Parser::isDeclarationSpecifier` is never called in case of C++. So I am not sure if we need to cover C++ specific constructs or not. But `unknown const *` is a good catch, I missed it for some reason. I will update the patch soon.


================
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;
----------------
aaron.ballman wrote:
> 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!).
There is weird behavior in case of ObjC range-based for loop. For example, in `for (NSString* key in xyz)`  token for `in` keyword is of type `Identifier` by the call of `Parser::isDeclarationSpecifier`. So I decided to exempt it in first version and discuss it in review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137020/new/

https://reviews.llvm.org/D137020