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

regression: reached recursion limit #125200

Open
BoxyUwU opened this issue May 17, 2024 · 9 comments
Open

regression: reached recursion limit #125200

BoxyUwU opened this issue May 17, 2024 · 9 comments
Labels
P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@BoxyUwU
Copy link
Member

BoxyUwU commented May 17, 2024

[INFO] [stdout] error: reached the recursion limit while instantiating `<(&mut combine::parser::sequence::Skip<combine::parser::combinator::Try<combine::parser::error::Expected<..., ...>>, ...>, ..., ...) as Parser<...>>::parse_mode::<...>`
[INFO] [stdout]    --> /opt/rustwide/cargo-home/registry/src/index.crates.io-6f17d22bba15001f/combine-4.6.2/src/parser/combinator.rs:236:15
[INFO] [stdout]     |
[INFO] [stdout] 236 |         match self.0.parse_mode(mode, input, state) {
[INFO] [stdout]     |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[INFO] [stdout]     |
@BoxyUwU BoxyUwU added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels May 17, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 17, 2024
@compiler-errors compiler-errors added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label May 17, 2024
@BoxyUwU BoxyUwU added this to the 1.79.0 milestone May 17, 2024
@compiler-errors
Copy link
Member

compiler-errors commented May 17, 2024

searched nightlies: from nightly-2024-02-15 to nightly-2024-05-17
regressed nightly: nightly-2024-03-22
searched commit range: 1388d7a...0ad927c
regressed commit: df8ac8f

bisected with cargo-bisect-rustc v0.6.8

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2024-02-15 

@compiler-errors
Copy link
Member

cc @RalfJung, @oli-obk

@compiler-errors compiler-errors removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label May 17, 2024
@RalfJung
Copy link
Member

Did this crate previously succeed to build in both debug and release configurations? My suspicion is that debug builds were already broken, and #122568 just made release builds the same, which is the entire intention of that PR.

@lqd
Copy link
Member

lqd commented May 17, 2024

Did this crate previously succeed to build in both debug and release configurations?

I believe so. It builds on stable in both debug and release, tests pass in both debug in release. But fails to build on beta with the OP.

@RalfJung
Copy link
Member

Okay, interesting.

Unfortunately I know nothing about this entire recursion limit stuff. There's this recursion_depth_reset in the collector, no idea what it does. There are hardly any comments.

If the recursion limit is increased, does the crate build again?
I guess it is expected that compiler refactoring can sometimes make a crate that was barely under the recursion limit, go over. Do we have policy for that?

@lqd
Copy link
Member

lqd commented May 17, 2024

If the recursion limit is increased, does the crate build again?

Yes, I tried e.g. 190, and it worked.

I guess it is expected that compiler refactoring can sometimes make a crate that was barely under the recursion limit, go over. Do we have policy for that?

I don't know but I believe I've seen it happen before yeah. So maybe this is fine, I'm not sure? Let's see what others think.

The slightly annoying part here I guess, is that the error mentions hitting the recursion limit, but in this instance doesn't say how to increase it. (Maybe we only do so if the #![recursion_limit] attribute is present in the source already, and we just add a note to double it.)

@saethlin
Copy link
Member

Maybe we only do so if the #![recursion_limit] attribute is present in the source already

I am pretty sure I've been prompted to add the attribute when it's not already in my source

@apiraino
Copy link
Contributor

I'd also like to get a feeling of how wide this change could be for other crates. And if it should be given visibility in the release notes (as a head-up when new stable lands)

@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 28, 2024
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 31, 2024
@Mark-Simulacrum Mark-Simulacrum added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants