trait_selection: fix assumptions-on-binders diagnostics

[Dnreikronos]

View all comments

fixes #157732

-Zassumptions-on-binders was losing the origin of solver region constraints. By the time regionck or borrowck reported them, every constraint received the containing item span, so diagnostics pointed at an entire function or const. Some paths were also still emitting placeholder text (:3 and meoow :c).

This keeps canonical solver responses and ExternalConstraintsData span-free, so source locations do not participate in candidate equality or caching. When a response is applied, EvalCtxt::origin_span is attached to each atomic solver RegionConstraint stored in InferCtxt. Late region conversion then uses the span attached to each constraint, including for ambiguity diagnostics.

The affected paths now report higher-ranked lifetime bound could not be satisfied at the type use that introduced the failing constraint. UI coverage includes two failures from one InferCtxt to ensure their origins remain distinct. Unit coverage checks that the spanned evaluator stays in semantic parity with the type-ir evaluator and preserves the first ambiguity origin span.

Future work: carry the failed outlives predicate or originating binder far enough through this path to name the exact bound that failed, rather than only pointing at its origin.

[rustbot]

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

[rustbot]

r? @folkertdev

rustbot has assigned @folkertdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 17 candidates

[folkertdev]

r? types

[lcnr]

r? BoxyUwU

We intend to fully rework the way these diagnostics work and the errors are currently intentionally bad as they don't even have span information (they point to the whole containing function).

I think keeping the status quo is preferable as a very explicit "this is still very WIP" and would personally prefer to close this PR

[rustbot]

BoxyUwU is currently at their maximum review capacity.
They may take a while to respond.

[Dnreikronos]

r? BoxyUwU

We intend to fully rework the way these diagnostics work and the errors are currently intentionally bad as they don't even have span information (they point to the whole containing function).

I think keeping the status quo is preferable as a very explicit "this is still very WIP" and would personally prefer to close this PR

thanks, that makes sense. i was mostly looking at this as remove the placeholder text, but imo you're right that the nicer message is a bit misleading if the span is still basically wrong.

i'll wait for boxyuwu's opinion before closing the pr. if i poke at this again later, i think i'd rather start by carrying more of the failed outlives predicate / binder info through first, then make the diagnostic nicer once it has enough data to not lie.

[BoxyUwU]

I think I agree with lcnr here 👍 Would you be interested in trying to do a more involved refactor here and figure out properly spanned diagnostics for -Zassumptions-on-binders? i.e. make it so we aren't just pointing to the whole item which the constraint originates from.

I think this would be really useful to have done and I won't get the time to do it for a while because of other priorities (getting std/core to compile and then custom test suite).

[Dnreikronos]

I think I agree with lcnr here 👍 Would you be interested in trying to do a more involved refactor here and figure out properly spanned diagnostics for -Zassumptions-on-binders? i.e. make it so we aren't just pointing to the whole item which the constraint originates from.

I think this would be really useful to have done and I won't get the time to do it for a while because of other priorities (getting std/core to compile and then custom test suite).

Of course!
I will do my best!

[BoxyUwU]

sick :3 it's probably a more involved change than your previous PRs so it's probably worth us chatting on zulip about it as you're working on it ✨ I would probably start by looking into where the spans for normal region errors come from (i.e. without -Zassumptions-on-binders) and then look into how to replicate that for -Zassumptions-on-binders

[BoxyUwU]

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[Dnreikronos]

@rustbot ready

[BoxyUwU]

ok sick this is what I had in mind :) thanks, I think this will merge conflict with your other PR so lets wait for that to merge first

View changes since this review

[BoxyUwU]

Suggested change

	/// A solver region constraint together with the span that caused each atomic constraint.
	/// A solver region constraint together with the span that caused each leaf constraint.

[Dnreikronos]

Just adjusted, Boxy :)

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[rust-bors]

☔ The latest upstream changes (presumably #160982) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.