parser: change warning into an error on T<A=B, C>

[birkenfeld]

part of #32214

This seems to be the obvious fix, and the error message is consistent with all the other parser errors ("expected x, found y").

[rust-highfive]

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

[Manishearth]

From the bug:

In either case, we should explain why there is an error here - because you can't have unbound type variables after bound ones.

[birkenfeld]

I saw that, but what makes this one special compared to other syntax errors?

[Manishearth]

We should still emit the expected blah found blah, but also emit extra help text that's in readable English instead of parsertongue 😛

[birkenfeld]

Not that I disagree, but let me ask again: why is this parse error so special compared to the dozens of others which could get the same treatment?

I'd propose to start with this change to get rid of the strange warning, and to stop accepting invalid parses.

[Manishearth]

why is this parse error so special compared to the dozens of others which could get the same treatment?

They should get the same treatment!

@bors r+ , in that case

however this does not fix #32214 completely 😄

[bors]

📌 Commit 98d991f has been approved by Manishearth

[durka]

I suggested a more helpful error (and then deleted the comment, but I forget why) but others said the parser should be pure and unaware of language constructs.

[Manishearth]

I disagree. This is diagnostics, which is a very real concern. IIRC our parser still doesn't point out where you may have forgotten a semicolon, instead spitting out a cryptic parsertongue expected/found message. This is not a trivial thing to fix, but it's a pain point for people new to the language. In general the parsertongue errors don't help much.

I don't see any concrete benefit to keeping language constructs out of the parser -- it's more of an ideal, and ideals should not make concrete concerns like diagnostics suffer. As long as it's limited to where the errors are emitted, IMO we should be okay.

[durka]

Parseltongue? 🐍

[Manishearth]

No, parsertongue 😛

The language of the parser. tokens and such. Not necessarily readable/understandable, i.e. in the case of the error the parser emits here.

[durka]

Yeah I mean, this code is in a function called parse_generic_values_after_lt so it seems a bit academic to pretend we don't know what construct it is.

And I'm in favor of renaming parsertongue to parseltongue :)

[birkenfeld]

One argument that could be made against adding these diagnostics is that it becomes much harder to migrate them to a parser automatically generated from a grammar. I don't know if this is planned (or even possible anymore)?

On the other hand, there are parsers that don't formulate expectations in terms of tokens, but in terms of constructs (e.g. "expected associated type or >, found identifier"). This could already help a lot.