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 generic default type assignment panic #6087

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

MarcusGrass
Copy link
Contributor

@MarcusGrass MarcusGrass commented Feb 22, 2024

Fixes #6052

The cause of the issue is when there's an = type assignment with the generics that overflows a line.

The budget given to def.rewrite is too low, this causes a cascading Option::None which eventually is unwrapped and causes the panic.

One more interaction caused another Option::None, the first was the formatting mentioned above, the second is that overflow.rs hardcodes to leave the last entry, which causes nothing to be written on a single entry, as was my test-case. I changed that logic to leave_last if greater than 1 length. I'm not familiar enough with the codebase to tell whether that could cause problems that aren't covered by tests.

I tried out two solutions to the formatting issue:

  1. Just give a budget of usize::MAX, then the user gets a line too long instead of a panic.
  2. Split the eq_str lhs and rhs into separate lines if it's too long, if it's still too long after that the user gets a line too long. Still usinge usize::MAX as budget.

Ending up going with the second one, since that seems nicer, it's very easy to change to the first one though.

E: Added the reported example as is as a test.
Also ran it on tokio, cause it has a bunch of code, to check for unintended consequences, not the best way to check for those apart from the tests, but it's something

@MarcusGrass MarcusGrass changed the title Issue 6052 Fix generic default type assignment panic Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Format crashes unexpectedly.
2 participants