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