Parse qualified paths in type params and provide custom diagnostic

[estebank]

Fix #26271.

[rust-highfive]

r? @michaelwoerister

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

[Centril]

When #52662 (cc) becomes stable we can suggest that instead.

[michaelwoerister]

@petrochenkov Do you have opinions on this? Do you even want to review it maybe? :)

[petrochenkov]

I'm generally unhappy about parsing random specific constructions in random positions just because someone made a ticket.

Perhaps the "consider adding an explicit lifetime bound ::TrSubtype: 'a" suggestion should instead mention where and point to a better code location, so mistakes like that don't have opportunity to occur.

[estebank]

@petrochenkov beyond the specifics of this ticket in particular, I believe the parser should be able to parse a superset of valid code for things that are either correct in other languages (a or b -> a || b) or that seem like valid syntax given how other parts of the syntax work (let x = [0]; x.0/let x: &'a usize = &'a 3;). Writing a qpath in a type argument list seems like something that fits this later behavior, something someone familiar with Rust syntax but not all its intricacies would try. Also I would point out that if there's one ticket it probably means there is a larger multiple of people attempting this and being confused. It is true that the chain of thought that ended in this code is no longer as problematic, but would still want the compiler to catch it.

[Centril]

Also I would point out that if there's one ticket it probably means there is a larger multiple of people attempting this and being confused.

I don't have an opinion on the specifics of this PR; but in general, is this really true?

[estebank]

@Centril I have a few dozen coworkers writing Rust. Anecdotally, I have only seen three people in a little less than a year file a rustc bug for something they spent a day or more trying to figure out, and two of them only did so because I encouraged them to do so. The amount of times they have approached others to ask what the hell the compiler is saying is somewhere around 50x.

I can certainly see that there are some really obscure corner cases (like this one) that might not be worth complicating the parser for, but any ticket that has either "+1" style comments or duplicates tells me that the amount of people actually encountering that problem is probably >100, and those numbers are only going to increase as the community expands. Not every diagnostic is born equal, but just as much as following the Pareto curve let's us focus most of our attention to the cases that come up 80% the time, I think we should give some attention to the long tail cases, particularly if .

I'm also empathetic to needing to clean up the happy path code and make diagnostics code unobtrusive. This is something that myself and others are planning to work on once we have a large enough chunk of time to do so: it's at the top of our priorities for this year.

[Centril]

I can certainly see that there are some really obscure corner cases (like this one) that might not be worth complicating the parser for, but any ticket that has either "+1" style comments or duplicates tells me that the amount of people actually encountering that problem is probably >100, and those numbers are only going to increase as the community expands.

@estebank If there are duplicates and "me too" comments on such diagnostics issues then I think there's a strong case for the improvement benefiting many people. I mainly think that it's not necessarily true that if there's a single issue reporting something obscure, that it will affect many folks and that it therefore many not deserve special attention.

I'm also empathetic to needing to clean up the happy path code and make diagnostics code unobtrusive. This is something that myself and others are planning to work on once we have a large enough chunk of time to do so: it's at the top of our priorities for this year.

❤️

[petrochenkov]

@estebank

I believe the parser should be able to parse a superset of valid code

Agree entirely.

beyond the specifics of this ticket in particular

My concerns are mostly about particularities and specific PRs, not about improving error recovery in general.

Writing a qpath in a type argument list seems like something that fits this later behavior

Here's where I disagree.
Why does qpaths get special treatment, but e.g. struct Bar<'a, T, "hello"> doesn't?
Because another diagnostics recommended it, but didn't recommend "hello".
From my point of view it would be better to fix the root issue (that another diagnostics positioning), than consequences.

Also I would point out that if there's one ticket it probably means there is a larger multiple of people attempting this and being confused.

That's arguable.
Some people report any perceived minor issue, some don't report anything unless it ruins their business (and, of course, there's a whole spectre in between).
So, an individual issue without confirmations from other people may turn out a whim of an enthusiastic reporter.
In case of #26271 there is one confirmation, so I'm ready to believe that the issue is real, but would still prefer to address it in other way.

Even if an issue is real and closing it is a positive, we should still try balance it with less noticeable and more long term goals, and attempt to 1) fix it as non-intrusively as possible (only wording, ideally) and 2) generalize to increase the positive effect (while not increasing intrusiveness).

In case of this PR a wording-only solution (mentioning where clauses) + perhaps adjusting position of the suggestion would be the more balanced solution, IMO.

(I'm not going to go on a crusade and block PRs that provide unbalanced solutions to diagnostic issues, but I'll probably still going to nag people about this from time to time, because the priorities need to be adjusted more in favor of the compiler maintainability.)

[Centril]

@estebank Perhaps leave a FIXME for the replacement when the RFC stabilizes?

[michaelwoerister]

r? @petrochenkov, if you don't mind.

[petrochenkov]

r? @petrochenkov, if you don't mind.

Oh no.

[estebank]

@petrochenkov I have PR I'm working on to split impl Parser in two/three traits, at least one for happy path and one exclusively for diagnostics. Would you prefer if we revisited this (and the other PR) after I have done that?

[petrochenkov]

Yeah, that would be nice, this is not an urgent issue.
(Closing.)