Skip to content

Conversation

@ShoyuVanilla
Copy link
Member

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 23, 2025
// This is skipped for the old solver: attempting trait solving there can
// trigger an overflow, which is a fatal error in the old solver but is
// treated as mere ambiguity by the next solver.
if self.next_trait_solver()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, checking function's bounds against the expected input tys regresses tests/ui/higher-ranked/trait-bounds/hrtb-doesnt-borrow-self-2.rs in the old solver.

impl<'a> Stream for &'a mut Repeat {
type Item = &'a u64;
fn next(self) -> Option<Self::Item> {
Some(&self.0)
}
}
pub struct Map<S, F> {
stream: S,
func: F,
}
impl<'a, A, F, T> Stream for &'a mut Map<A, F>
where
&'a mut A: Stream,
F: FnMut(<&'a mut A as Stream>::Item) -> T,
{
type Item = T;
fn next(self) -> Option<T> {
match self.stream.next() {
Some(item) => Some((self.func)(item)),
None => None,
}
}
}
pub struct Filter<S, F> {
stream: S,
func: F,
}
impl<'a, A, F, T> Stream for &'a mut Filter<A, F>
where
for<'b> &'b mut A: Stream<Item = T>, // <---- BAD
F: FnMut(&T) -> bool,
{
type Item = <&'a mut A as Stream>::Item;
fn next(self) -> Option<Self::Item> {
while let Some(item) = self.stream.next() {
if (self.func)(&item) {
return Some(item);
}
}
None
}
}
pub trait StreamExt
where
for<'b> &'b mut Self: Stream,
{
fn mapx<F>(self, func: F) -> Map<Self, F>

Evaluating the bounds for<'b> &'b mut Self: Stream on method StreamExt::mapx causes overflow while trying to consider the candidate &'_ mut Map<Map<Map<...>, _>, _>, _> and this ends up with a fatal error 😢

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this PR is currently doing multiple somewhat distinct changes in the same commit. Please split them up into separate commits (or PRs even)

View changes since this review

self.param_env,
ty::ClauseKind::WellFormed(ty.into()),
));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention was to check well-formedness of expected input tys and I thought this would do the thing as they are equated with formal input tys.
But it might be more correct to check them separately outside the fudge

@ShoyuVanilla
Copy link
Member Author

I feel like this PR is currently doing multiple somewhat distinct changes in the same commit. Please split them up into separate commits (or PRs even)

Yeah, I tried to do two things, checking well-formedness of expected input tys and whether they meet the callee's where bounds 😅

I'll break them into three commits, one for adding tests from related issues, wf check, and where bound check, with blessed test output to visualize the influence of each changes.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 15, 2026

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

@rustbot
Copy link
Collaborator

rustbot commented Jan 15, 2026

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.

@ShoyuVanilla ShoyuVanilla requested a review from lcnr January 15, 2026 16:36
@ShoyuVanilla
Copy link
Member Author

Sorry for the ping 😅 I've split the changes into multiple commits and blessed tests for each commit. Would this be okay?

ocx.try_evaluate_obligations().is_empty()
});
well_formed.then_some(expected_input_tys)
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why and then instead of filter?

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the second change isn't necessary to avoid any breakage when switching solvers, is it?

I think to fix rust-lang/trait-system-refactor-initiative#259 we should just do f5acf22

I think these sorts of improvements are generally good, but don't like improvements which are only limited to the new solver. I would drop the second commit and then get back to that one once the new solver is stable

View changes since this review

@ShoyuVanilla
Copy link
Member Author

Yeah, moving generalization outside of the fudge fixes the issue but it regresses back an improvement made by next-solver as said in #t-types/trait-system-refactor > diesel benchmark doesn't compile @ 💬

I'll try combining it with the third commit of this PR and if it doesn't fix the regression, I think this should be closed and revisited once the new solver is stabilized 😄

@ShoyuVanilla
Copy link
Member Author

Yeah, moving generalization outside of the fudge fixes the issue but it regresses back an improvement made by next-solver as said in #t-types/trait-system-refactor > diesel benchmark doesn't compile @ 💬

I'll try combining it with the third commit of this PR and if it doesn't fix the regression, I think this should be closed and revisited once the new solver is stabilized 😄

Oops, the third commit + f5acf22 still regresses back tests/ui/traits/next-solver/lazy-nested-obligations-2.rs 😅 Closing this for now

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants