shafik marked an inline comment as done. shafik added inline comments.
================ Comment at: clang/lib/Parse/ParseTentative.cpp:1553-1554 return TPResult::Error; - if (Tok.isNot(tok::identifier)) + if (NextToken().isNot(tok::identifier)) break; } ---------------- cor3ntin wrote: > rsmith wrote: > > This doesn't seem correct to me. If we had `scope::foo bar`, and we > > annotate `scope::foo` as a type, then this will get confused by the next > > token now being an (unrelated) identifier. This code is trying to detect if > > an annotation was performed, so I think it intended to check if the current > > token's kind has changed, like is done on line 1295. > The confusing bit is that Tok is always an annotated scope already here > (L1598), so TryAnnotateName should not modify that first token (unless > TryAnnotateTypeOrScopeTokenAfterScopeSpec can somehow replace the current > annot_cxxscope by another one, which i don't think can happen?) Ok using `tok::annot_cxxscope` also works and I agree it makes sense as well, `check-clang` also passes. So then is the assert below wrong? ``` // Annotated it, check again. assert(Tok.isNot(tok::annot_cxxscope) || NextToken().isNot(tok::identifier)); ``` It looks like it will work by accident for most cases b/c it checks `tok::annot_cxxscope` first. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134334/new/ https://reviews.llvm.org/D134334 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits