[PATCH] D134334: [Clang] Fix crash in isCXXDeclarationSpecifier when attempting to annotate template name

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D134334#3827895 <https://reviews.llvm.org/D134334#3827895>, @shafik wrote:

> In D134334#3805590 <https://reviews.llvm.org/D134334#3805590>, @erichkeane wrote:
>
>> I have no real idea what is going on here, the parser isn't an area where I spend much time.  Can you ELI5?
>
> I am going to try but perhaps fail to explain this in more detail and more clearly.
>
> IIUC we are trying to error recover, we get to
>
>   	  // Try to resolve the name. If it doesn't exist, assume it was
>             // intended to name a type and keep disambiguating.
>             switch (TryAnnotateName()) {
>
> At this point we know the current token is `::` and the next token is an identifier. We are trying to annotate the name and it could be a C++17 class template argument deduction case: https://en.cppreference.com/w/cpp/language/class_template_argument_deduction
>
> If we annotate it we should not be left with current token as `::` and the next token as an identifier, this is what the assert verifies here:
>
>   // Annotated it, check again.
>   assert(Tok.isNot(tok::annot_cxxscope) ||
>          NextToken().isNot(tok::identifier));
>
> So the line I modified says we should only break if we actually annotated and therefore the next token IS NOT an identifier b/c we would have advanced.

In that case: 1- could you document that?  2- is there some level of more-clear check you could do?  Someting like "if (NextToken().is(tok::annotateTok)"?  3- Should TryAnnotateTypeOrScopeToken give SOME sort of 'I successfully did the thing?' instead?


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

https://reviews.llvm.org/D134334