[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:5425
+    // node in AST for such cases which is good for AST readers.
+    if (IsUnknownTypedefName() && !getLangOpts().ObjC)
+      return true;
----------------
sammccall wrote:
> urazoff wrote:
> > 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.
> This looks very suspicious to me, for example in `a * b;`. isDeclarationSpecifier() is going to return true when pointing at `a`. In fact `a` may be either a type or an expression here.
> 
> Even if this right now this function never gets called for that case, it's not obvious why it wouldn't be in the future, or wouldn't be called for similar cases.
> 
The property I relied on is that token `a` in `a * b;` gets annotated before and default branch works. But I understand your concern, I am gonna update the patch soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137020