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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits