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

This comment has been minimized.

@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
Member Author

Choose a reason for hiding this comment

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

Yeah, this is very absurd 😅 I guess I was trying to do more things but just ended up something equivalent to 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
@lcnr
Copy link
Contributor

lcnr commented Jan 20, 2026

The first change is also old solver, is it not? That one seems cool and I think I'd like to FCP it rn

@ShoyuVanilla
Copy link
Member Author

Oh, I misread your comment 😅 You meant effd66a by the word second commit. Then I'll reopen this with it

@lcnr
Copy link
Contributor

lcnr commented Jan 20, 2026

I think 9f3e4d5 is clearly good and doesn't depend on nexct solver, I think effd66a should not land for now as it's limited to new solver for now

@ShoyuVanilla
Copy link
Member Author

Yeah, I understood what you meant by now but I should have written "without it" or "with the first commit" instead of "with it" 😅 Anyway, I'll try that once I get home

@ShoyuVanilla ShoyuVanilla reopened this Jan 20, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 20, 2026
@rustbot
Copy link
Collaborator

rustbot commented Jan 20, 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 changed the title Fix typecks on some spurious fudged function input expectation Do not use ill-formed input expectations from fudge when checking function calls Jan 20, 2026
@ShoyuVanilla
Copy link
Member Author

I've dropped the last commit and fixed the filter thing.

Unfortunately, the wf commit + f5acf22 still regresses back tests/ui/traits/next-solver/lazy-nested-obligations-2.rs 🤔

@rustbot ready

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
REPOSITORY                                   TAG       IMAGE ID       CREATED       SIZE
ghcr.io/dependabot/dependabot-updater-core   latest    63ba4a2ff346   12 days ago   775MB
=> Removing docker images...
Deleted Images:
untagged: ghcr.io/dependabot/dependabot-updater-core:latest
untagged: ghcr.io/dependabot/dependabot-updater-core@sha256:830c840ea4b8c27b6919bdd47c017030adeb538f226e1cdfd386490a622b9218
deleted: sha256:63ba4a2ff3467a98ac6c8610fece2cdc9893e2af6c18111e57618f8949749b82
deleted: sha256:77617fa265a7311b1c0502e01ff6157d25228d91f6cddc21d7901eb440d0adee
deleted: sha256:0bd8abdbb1f7c0453b4bc76cce6ecf0a3b9e4b5d0c8ceeb63652e91a63cab4a1
deleted: sha256:4e9913397e051a70593edc1662de159778ce1bf65132295639da0e119913b53f
deleted: sha256:3e54edb2fef4b1c1393a2928bfa1017c22cc578f5f3eef677580662e8d2e79ea
---
[WARNING] line 39: Delta is 0 for "x", maybe try to use `compare-elements-position` instead?

======== tests/rustdoc-gui/type-declation-overflow.goml ========

[ERROR] type-declation-overflow output:
Waiting failed: 30000ms exceeded
stack: TimeoutError: Waiting failed: 30000ms exceeded
    at new WaitTask (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/puppeteer-core/lib/cjs/puppeteer/common/WaitTask.js:49:34)
    at IsolatedWorld.waitForFunction (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/puppeteer-core/lib/cjs/puppeteer/api/Realm.js:25:26)
    at CdpFrame.waitForFunction (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/puppeteer-core/lib/cjs/puppeteer/api/Frame.js:583:43)
    at CdpFrame.<anonymous> (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/puppeteer-core/lib/cjs/puppeteer/util/decorators.js:109:27)
    at CdpPage.waitForFunction (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/puppeteer-core/lib/cjs/puppeteer/api/Page.js:1445:37)
    at runAllCommands (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/browser-ui-test/src/index.js:429:28)
    at async innerRunTestCode (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/browser-ui-test/src/index.js:707:21)
    at async innerRunTests (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/browser-ui-test/src/index.js:646:17)
    at async runTests (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/browser-ui-test/src/index.js:835:27)



<= doc-ui tests done: 144 succeeded, 1 failed, 0 filtered out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants