Conversation
Signed-off-by: 0xull <mrikehchukwuka@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #23337.
rust-analyzer analysis-stats .panics while const-evaluating a tuple struct constructor called with more arguments than the struct has fields:Evaluator::construct_with_layoutwrites each incoming operand into the result buffer at the layout offset for that operand's index. the operands come from the mir call terminator while the layout comes from the adt definition, and nothing checks that the two agree on how many fields there are. so, once the call supplies more operands than the layout has fields,FieldsShape::offsetindexes its offsets vector past the end and panics.but the loop already handles the neighbouring failure, so the shape of the fix was somewhat decided for me. it returns
MirEvalError::InternalErrorwhen the byte range for an operand falls outside the result buffer, and this adds the matching check one line earlier for the field index itself. i usedFieldsShape::countfor the correct bound for all four field shapes, so this one comparison covers unions and arrays along with ordinary structs, covering all its six call sites.after the change the const fails to evaluate rather than taking the process down.
i would want to note one thing about the reproducer in the issue. so far, it contains two syntax errors and neither turns out to be load-bearing (though), because the same panic happens for a fixture that parses cleanly:
here, rustc still rejects that with E0061, so it is ill-typed rather than valid, but no parse recovery is involved. i used the well-formed version as the regression test since it pins down the actual precondition, and also i confirmed separately that the fixture from the issue passes as well.
ai disclosure: i triaged the panic (and its backtrace) with help by claude (web), but final code and commit
message are by me.