vsapsai added inline comments.
================ Comment at: clang/lib/Parse/ParseDecl.cpp:3076 DS.SetRangeEnd(Tok.getAnnotationEndLoc()); ConsumeAnnotationToken(); // The typename } ---------------- rsmith wrote: > vsapsai wrote: > > Here we potentially can leave annotation token unconsumed. But I wasn't > > able to create a test case that would trigger a problem at this point. > I think you're talking about the `break` a few lines back? > > I think we actually get away with this, but only because `SetTypeSpecType` > can only fail if there was already a type specifier, which we checked for on > line 3019, so `isInvalid` is never `true`. You are right, I was talking about `break` on the line 3071. ================ Comment at: clang/lib/Parse/ParseDecl.cpp:3148 DS.SetRangeEnd(Tok.getAnnotationEndLoc()); ConsumeAnnotationToken(); // The typename ---------------- rsmith wrote: > vsapsai wrote: > > We didn't consume the annotation token because of `break` on `isInvalid` a > > few lines above. > I wonder if our error recovery would be improved by changing line 3134 to just > > ```if (DS.hasTypeSpecifier())``` > > (which would likewise render the `break` here unreachable given the current > rules enforced by `SetTypeSpecType`). Change in if condition fixes the assertion too and the diagnostics are ``` repro.cpp:1:18: error: expected a qualified name after 'typename' class A typename A; ^ repro.cpp:1:18: error: expected unqualified-id 2 errors generated. ``` With the currently reviewed change diagnostics are ``` repro.cpp:1:18: error: expected a qualified name after 'typename' class A typename A; ^ repro.cpp:1:18: error: cannot combine with previous 'class' declaration specifier 2 errors generated. ``` This is for the file ``` $ cat repro.cpp class A typename A; ``` This small test isn't representable but I find showing next to each other > expected a qualified name > expected unqualified-id confusing and contradictory at the first glance. If there are no further comments, I'll commit the change as is (after adding a comment for ConsumeAnyToken). ================ Comment at: clang/lib/Parse/ParseDecl.cpp:3802 if (DiagID != diag::err_bool_redeclaration) - ConsumeToken(); + ConsumeAnyToken(); ---------------- rsmith wrote: > Maybe add a comment here saying we can get here with an annotation token > after an error? Will do. https://reviews.llvm.org/D44449 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits