cor3ntin added a comment.
================
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: {
----------------
aaron.ballman wrote:
> 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.
Funnily, the cases
```
case Sema::NC_VarTemplate:
case Sema::NC_FunctionTemplate:
case Sema::NC_UndeclaredTemplate:```
Are never traversed, We can remove them without breaking tests. the assert
doesn't do anything then. arguably `ClassifyName` needs further fix but that
might be more complicated surgery
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
