Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Prevent infinite recursion of bounds formatting #19020

Merged
merged 2 commits into from
Jan 25, 2025

Conversation

ShoyuVanilla
Copy link
Member

@ShoyuVanilla ShoyuVanilla commented Jan 24, 2025

Fixes #19007

I've inserted extra tracings in scip and found out the exact position causing the issue:

https://docs.rs/futures-util/latest/src/futures_util/stream/try_stream/try_concat.rs.html#40

impl<St> Future for TryConcat<St>
where
    St: TryStream,
    St::Ok: Extend<<St::Ok as IntoIterator>::Item> + IntoIterator + Default,
{
    type Output = Result<St::Ok, St::Error>;

    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        let mut this = self.project();

        Poll::Ready(Ok(loop {
            if let Some(x) = ready!(this.stream.as_mut().try_poll_next(cx)?) {
                      //^ This token
                if let Some(a) = this.accum {
                    a.extend(x)
                } else {
                    *this.accum = Some(x)
                }
            } else {
                break this.accum.take().unwrap_or_default();
            }
        }))
    }
}

the x on the above code has the type St::Ok and the bounds St::Ok: Extend<<**St::Ok** as IntoIterator>::Item> + IntoIterator + Default and this is causing the infinite recursion, as @glandium said

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 24, 2025
@@ -10349,3 +10349,45 @@ macro_rules! str {
"#]],
);
}

#[test]
fn regression_19007() {
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this an inlay hint test and put it beside the tests introduced by #18925 so we've got them in the same area?

let db = f.db;
let id = from_placeholder_idx(db, *idx);
let generics = generics(db.upcast(), id.parent);
let res = f.format_bounds_with(|f| {
Copy link
Member

Choose a reason for hiding this comment

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

I feel like we might wanna move this down into the if !bounds.is_empty() { branch body? Recursive bounds like these will be the minority and so we prevent triggering a heap alloc in the case where we aren't actually doing the different rendering here (but only realize so within this closure.

@Veykril Veykril added this pull request to the merge queue Jan 25, 2025
Merged via the queue into rust-lang:master with commit 90bf50c Jan 25, 2025
9 checks passed
@ShoyuVanilla ShoyuVanilla deleted the issues-19007 branch January 26, 2025 04:08
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[scip] "thread 'main' has overflowed its stack" since nightly-2025-01-21
3 participants