refactor: keep attr-only statements and cloned lets printable#1457
Open
refactor: keep attr-only statements and cloned lets printable#1457
Conversation
fw-immunant
approved these changes
Nov 17, 2025
Contributor
fw-immunant
left a comment
fw-immunant
left a comment
There was a problem hiding this comment.
Seems reasonable. Is there an easy way to observe the change in behavior? It would be nice to have a test case if possible.
ahomescu
reviewed
Nov 19, 2025
| /// Pick the span we should attach to a freshly minted statement. | ||
| /// | ||
| /// We want new statements produced by transforms such as `sink_lets` or | ||
| /// `fold_let_assign` to retain the source span of the code they were cloned |
Contributor
There was a problem hiding this comment.
Could we just fix the transforms then? Why couldn't they pass in the spans?
Contributor
Author
oinoom
force-pushed
the
fix.printparse-attr-spans
branch
from
86b24c2 to
f294de7
Compare
Contributor
Author
|
@ahomescu @fw-immunant I'm trying a different approach in #1469 based on this comment. If that looks good, I'll merge it and close this one
|
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
3 participants
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.
Several transforms (
sink_lets,remove_redundant_let_types) were panicking whenever the rewrite engine had to reparse snippets that no longer looked like full Rust statements.Two cases showed up when testing:
#[derive]lines, the transform temporarily splices only the attribute text (for example just#[derive(Copy, Clone)]) while the enclosing struct body is rewritten at a higher level. Rust’s parser refuses that input, soPrintParse<Stmt>panicked before recovery finishedletbindings inherit the builder’s defaultDUMMY_SP, so pretty-printing emits an empty string even though the original code had a real initializer. The AST/text round-trip fails and subsequent rewrites abortThis PR:
PrintParse<Stmt>up front, reparses them by temporarily appending a dummy struct, and returns aStmtKind::Emptythat carries the combined attribute span, which keeps the rewriter from panicking while the surrounding struct/item is printed elsewhereDUMMY_SP. Newly cloned locals/exprs retain their original source ranges, sorewrite_at_implcan pull real snippets instead of an empty buffer