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

Recursion limit tripped in doc but not in other profiles #100991

Closed
TaKO8Ki opened this issue Aug 25, 2022 · 11 comments · Fixed by #101039
Closed

Recursion limit tripped in doc but not in other profiles #100991

TaKO8Ki opened this issue Aug 25, 2022 · 11 comments · Fixed by #101039
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@TaKO8Ki
Copy link
Member

TaKO8Ki commented Aug 25, 2022

The diesel benchmark has been broken in some recent commit: https://perf.rust-lang.org/status.html.

ref: zulip comment

@rylev
Copy link
Member

rylev commented Aug 25, 2022

The error seems to be caused #100757 (cc @ouz-a). Curiously, this only happens in doc.

Here's the error:

 Documenting diesel v1.4.8 (/home/rylevick/code/rustc-perf/collector/benchmarks/diesel-1.4.8)
error[E0275]: overflow evaluating the requirement `<_ as query_source::Column>::Table`
    |
    = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`diesel`)
note: required for `&expression::operators::Eq<_, _>` to implement `insertable::Insertable<_>`
   --> src/expression/operators.rs:376:21
    |
376 | impl<'a, T, Tab, U> Insertable<Tab> for &'a Eq<T, U>

The line where the error triggers is:

impl<'a, T, Tab, U> Insertable<Tab> for &'a Eq<T, U> // <--- HERE
where
    T: Copy,
    Eq<T, &'a U>: Insertable<Tab>,
{
    type Values = <Eq<T, &'a U> as Insertable<Tab>>::Values;

    fn values(self) -> Self::Values {
        Eq::new(self.left, &self.right).values()
    }
}

You can reproduce this issue by:

  • cloning the rustc-perf repo
  • going to diesel-1.4.8 benchmark in that repo
  • installing commit a72b0e79ba45f05b339b45034996086bf10d3c26 using rustup-toolchain-install-master
  • running cargo +a72b0e79ba45f05b339b45034996086bf10d3c26 doc

@TaKO8Ki TaKO8Ki added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Aug 25, 2022
@rylev rylev changed the title diesel benchmark has been broken Recursion limit tripped in doc but not in other profiles Aug 25, 2022
@ouz-a
Copy link
Contributor

ouz-a commented Aug 25, 2022

How can I run this(rustc-perf repo) via locally built rustc? @rylev

@weiznich
Copy link
Contributor

#100620 might be related as well?

@ouz-a
Copy link
Contributor

ouz-a commented Aug 25, 2022

That indeed looks related, so #100705 should fix this ?

@rylev
Copy link
Member

rylev commented Aug 25, 2022

The comment here also makes it look like #100705 will fix this.

@rylev
Copy link
Member

rylev commented Aug 26, 2022

Unfortunately, #100705 did not fix the issue.

Using the build bors creates after merge 983f4da, cargo +983f4daddf238d114c4adc4751c5528fc6695a5a doc leads to the same error as above.

@ouz-a to reproduce this, you just need to clone the rustc-perf repo and cd into collector/benchmarks/diesel-1.4.8. Then run rustup-toolchain-install-master 983f4daddf238d114c4adc4751c5528fc6695a5a and cargo +983f4daddf238d114c4adc4751c5528fc6695a5a doc

@ouz-a
Copy link
Contributor

ouz-a commented Aug 26, 2022

Thanks, I will try to push a fix today.

@compiler-errors
Copy link
Member

I'm just gonna revert my original PR that introduced this overflow until I can create a version that works with diesel, @ouz-a, given the perf regression of my "fix" and the remaining existence of this issue, so don't waste too much time on this.

@ouz-a
Copy link
Contributor

ouz-a commented Aug 26, 2022

I already found a fix 😅

@jyn514
Copy link
Member

jyn514 commented Sep 20, 2022

cc #81091

@jyn514
Copy link
Member

jyn514 commented Sep 28, 2022

I've started on minimizing this in https://github.com/jyn514/diesel/tree/mangled-diesel-ice, with the goal of removing the rustdoc special casing once I finish #81091. If someone could finish it up that would be extremely helpful - I think it's related to one of the impls on QueryFragment, if I define an identical trait in mod reproduce it doesn't overflow anymore.

You can reproduce the overflow with rustup-toolchain-install-master 983f4daddf238d114c4adc4751c5528fc6695a5a && cargo +983f4daddf238d114c4adc4751c5528fc6695a5a doc -p diesel --no-default-features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants