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

New reached the type-length limit while instantiating error appearing on 1.81.0-nightly (aa1d4f682 2024-07-03) #127346

Closed
TheLostLambda opened this issue Jul 4, 2024 · 6 comments · Fixed by #127670
Labels
T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@TheLostLambda
Copy link

TheLostLambda commented Jul 4, 2024

Some of my CI started failing today, but only for the workflows relying on the nightly toolchain — you can get a closer look at the error and the code causing it here (as well as see that things work fine on stable still):
https://github.com/TheLostLambda/pg-pipeline/actions/runs/9800551201

Version it worked on

❯ rustc +nightly-2024-07-03 --version --verbose
rustc 1.81.0-nightly (6292b2af6 2024-07-02)
binary: rustc
commit-hash: 6292b2af620dbd771ebb687c3a93c69ba8f97268
commit-date: 2024-07-02
host: x86_64-unknown-linux-gnu
release: 1.81.0-nightly
LLVM version: 18.1.7

Version with regression

❯ rustc +nightly-2024-07-04 --version --verbose
rustc 1.81.0-nightly (aa1d4f682 2024-07-03)
binary: rustc
commit-hash: aa1d4f6826de006b02fed31a718ce4f674203721
commit-date: 2024-07-03
host: x86_64-unknown-linux-gnu
release: 1.81.0-nightly
LLVM version: 18.1.7

@rustbot modify labels: +regression-from-stable-to-{nightly} -regression-untriaged

@TheLostLambda TheLostLambda added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Jul 4, 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 Jul 4, 2024
@apiraino
Copy link
Contributor

apiraino commented Jul 5, 2024

The error in the CI logs I read is:

error: reached the type-length limit while instantiating `<{closure@final_parser<'_, ChemicalComposition<'_>, {closure@nom::combinator::into<&str, ..., ..., ..., ..., ...>::{closure#0}}, ...>::{closure#0}} as FnMut<...>>::call_mut`
  --> crates/polychem/src/atoms/chemical_composition.rs:20:9
   |
20 |         parser(formula.as_ref()).map_err(|e| Box::new(e.into()))
   |         ^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: consider adding a `#![type_length_limit="18554191"]` attribute to your crate
   = note: the full type name has been written to '/home/runner/work/pg-pipeline/pg-pipeline/target/debug/deps/polychem-8e9c97e0fbc5d0c1.long-type.txt'

I think it started in #125507 and it seems to be intended as it is labeled with relnotes as a breaking change to be announced in the next release of the stable compiler.

(is the above correct @compiler-errors ?)

@rustbot label -I-prioritize -regression-untriaged +T-types

@rustbot rustbot added T-types Relevant to the types team, which will review and decide on the PR/issue. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels Jul 5, 2024
@compiler-errors
Copy link
Member

@TheLostLambda: you should probably just add #![type_length_limit="18554191"] to your crate. You're producing some really large types in your crate 😄

@TheLostLambda
Copy link
Author

TheLostLambda commented Jul 6, 2024

@compiler-errors Wait until you see what I got from the other crate in my project:
image

In all seriousness though, I don't think I've been doing anything particularly unusual, but I think that noms parser-combinator style just leads to some massive types (perhaps similar to the itertools crate). Has the impact of (re)adding this limit been tested on other crates that depend on nom and itertools? And how many bits are used to store that number? I think I'm already filling at least 48?

Perhaps more importantly, even after adding those attributes, I can't get cargo build to work! After adding the increased limits, cargo test started working fine, but cargo build continues to complain and asks me to add the same limits that I already have! The means that, whilst I can run tests on my crate using the nightly compiler I currently cannot get it to build, bench, or run! Let me know if I'm just doing something silly, but my crate seems fully broken with the current nightly limit implementation!

It also still fails to build on CI: https://github.com/TheLostLambda/pg-pipeline/actions/runs/9818433944/job/27110969365

EDIT: Seeing the workaround that rust-itertools/itertools#945 used to fix one of the itertools issues is a bit frightening as well — if that's meant to improve before the next-solver is made the primary solver, then perhaps adding this type-length limit should either be set way higher or wait until then?

@saethlin
Copy link
Member

saethlin commented Jul 6, 2024

cargo build continues to complain and asks me to add the same limits that I already have!

The attributes need to be applied in the crate which is being compiled, not the crate that contains the offending span. The diagnostic makes this a bit confusing.

error: reached the type-length limit while instantiating `<{closure@nom_miette::final_parser<'_, ChemicalComposition<'_>, {closure@nom::combinator::into<&str, ..., ..., ..., ..., ...>::{closure#0}}, ...>::{closure#0}} as FnMut<...>>::call_mut`
  --> /tmp/pg-pipeline/crates/polychem/src/atoms/chemical_composition.rs:20:9
   |
20 |         parser(formula.as_ref()).map_err(|e| Box::new(e.into()))
   |         ^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: consider adding a `#![type_length_limit="18554191"]` attribute to your crate
   = note: the full type name has been written to '/tmp/pg-pipeline/target/debug/deps/molmass-93fc4367e4584c5b.long-type.txt'

error: could not compile `pg-pipeline` (bin "molmass") due to 1 previous error

In this case, the attribute needs to be added to src/bin/molmass.rs.

After applying this diff, cargo build succeeds:

diff --git a/src/bin/molmass.rs b/src/bin/molmass.rs
index 8e90189..f2c47ac 100644
--- a/src/bin/molmass.rs
+++ b/src/bin/molmass.rs
@@ -1,3 +1,5 @@
+#![type_length_limit = "18554191"]
+
 use miette::{Diagnostic, GraphicalReportHandler, GraphicalTheme};
 use once_cell::sync::Lazy;
 use polychem::{AtomicDatabase, Charged, ChargedParticle, ChemicalComposition, Massive, Result};
diff --git a/src/bin/pgmass.rs b/src/bin/pgmass.rs
index 71efb62..4f02b8e 100644
--- a/src/bin/pgmass.rs
+++ b/src/bin/pgmass.rs
@@ -1,3 +1,5 @@
+#![type_length_limit="30491060267835378"]
+
 use miette::{Diagnostic, GraphicalReportHandler, GraphicalTheme};
 use muropeptide::{Muropeptide, Result};
 use once_cell::sync::Lazy;

@saethlin saethlin removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jul 6, 2024
@TheLostLambda
Copy link
Author

TheLostLambda commented Jul 7, 2024

@saethlin I see! I do agree that's a bit unclear from the compiler diagnostic, and it would be nice to somehow replace the "your crate" with a more specific location in cases like these (especially when it's something a bit sneaky like a binary)!

Either way, thank you for the fix in my case, and I'll try to keep up with the conversation in #125507#![type_length_limit="30491060267835378"] is certainly somewhat horrifying...

Let me know if there is anything more I can do to help!

EDIT: Additionally, when it comes to improving the diagnostic (and I don't know how feasible this is), it might be nice if you were given the final size needed, instead up updating the limit once to get another error with a larger limit, then repeating until the errors stop.

@TheLostLambda
Copy link
Author

I've had to continue to add more #![type_length_limit="..."] markers around — now in the benchmark targets as well. I imagine this would mean than any consumers of my crate would also need to massively increase their type_length_limits to use my library?

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

Successfully merging a pull request may close this issue.

5 participants