aaron.ballman added inline comments.
================
Comment at: clang/lib/Parse/Parser.cpp:1886-1889
+ case Sema::NC_Concept:
case Sema::NC_VarTemplate:
case Sema::NC_FunctionTemplate:
case Sema::NC_UndeclaredTemplate: {
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > Would this change make sense, to validate that we're still consuming the
> > token in these three cases? I think it was an assumption we made
> > previously, but it's not clear to me if the code used to consume something
> > other than `<` as well.
> I'm not sure. You'll notice that using a var template without template
> argument does not trigger the assert. That's because `ClassifyName` will
> return `NC_NonType` instead of `NC_VarTemplate` in that case. I don't think
> that makes sense either, but that require more investigation. I do think
> however all template names not followed by arguments should produce a
> TemplateIdAnnotation so the change is not not correct :)
>
> If you insist, i can put the assert back, but I'm not sure the assert was
> testing the right thing.
I don't insist on adding the assert, it was more a matter of trying to figure
out whether the changes to the non-concept cases have changed parsing behavior
in some subtle way. My thinking is that an assert that fails (signaling we are
no longer consuming a token when we previously would have) would be easier for
us to debug than a mysterious parsing failure. But if that assert would be
wrong to add, then it makes me wonder what the test case is for it because then
we've changed parsing behavior.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146719/new/
https://reviews.llvm.org/D146719
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
