From e5f46c03c83c4020f8d56226c5c334001972b60e Mon Sep 17 00:00:00 2001 From: David Anekstein Date: Fri, 21 Nov 2025 18:18:56 -0500 Subject: [PATCH 1/3] refactor: fix span handling in sink_lets and remove_redundant_let_types Transforms that create new statement wrappers need to preserve source spans for the rewrite system to extract text during recovery. Changes: - Set stmt.span explicitly in transforms when wrapping locals - Extend Splice for Stmt to include attributes from inner nodes - Add fallback in rewrite_at_impl for macro-expanded code Fixes sink_lets and remove_redundant_let_types on libxml2. --- c2rust-refactor/src/rewrite/strategy/print.rs | 43 ++++++++++++++++++- c2rust-refactor/src/transform/vars.rs | 37 +++++++++++++++- 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/c2rust-refactor/src/rewrite/strategy/print.rs b/c2rust-refactor/src/rewrite/strategy/print.rs index 36f73fa609..495d040ef1 100644 --- a/c2rust-refactor/src/rewrite/strategy/print.rs +++ b/c2rust-refactor/src/rewrite/strategy/print.rs @@ -252,7 +252,24 @@ impl Splice for Ty { impl Splice for Stmt { fn splice_span(&self) -> Span { - self.span + // Extend statement span to include attributes on the inner node. + // + // Transforms like sink_lets set stmt.span = local.span, but attributes are + // stored on the inner Local/Item/Expr, not the Stmt wrapper. The pretty-printer + // outputs attributes, but if stmt.span doesn't cover them, the rewrite system + // can't extract the complete source text. + // + // extend_span_attrs() expands the span backward to include attributes that + // appear before the node in source, checking syntax contexts to avoid crossing + // macro boundaries. + let attrs = match &self.kind { + StmtKind::Local(local) => &local.attrs[..], + StmtKind::Item(item) => &item.attrs[..], + StmtKind::Expr(expr) | StmtKind::Semi(expr) => &expr.attrs[..], + StmtKind::MacCall(mac) => &mac.attrs[..], + StmtKind::Empty => &[], + }; + extend_span_attrs(self.span, attrs) } } @@ -674,7 +691,29 @@ fn rewrite_at_impl(old_span: Span, new: &T, mut rcx: RewriteCtxtRef) -> bool where T: PrintParse + RecoverChildren + Splice + MaybeGetNodeId, { - let printed = add_comments(new.to_string(), new, &rcx); + let mut printed = add_comments(new.to_string(), new, &rcx); + + // Fallback: extract source snippet when pretty-printing produces empty output. + // + // When transforms move macro-expanded code (e.g., from #[derive]), the span may + // point to generated code with no source file location. Pretty-printing such nodes + // can produce empty output, which causes problems during the reparse/recovery phase. + // + // If the printed text is empty, try extracting the source directly using + // splice_span() (which includes attributes). If source is available, use that + // for reparsing instead of the empty pretty-printed text. + if printed.trim().is_empty() { + if let Ok(snippet) = rcx + .session() + .source_map() + .span_to_snippet(new.splice_span()) + { + if !snippet.trim().is_empty() { + printed = snippet; + } + } + } + let reparsed = T::parse(rcx.session(), &printed); let reparsed = reparsed.ast_deref(); diff --git a/c2rust-refactor/src/transform/vars.rs b/c2rust-refactor/src/transform/vars.rs index 6b7cb756b5..d318bb8272 100644 --- a/c2rust-refactor/src/transform/vars.rs +++ b/c2rust-refactor/src/transform/vars.rs @@ -192,7 +192,18 @@ impl Transform for SinkLets { Some(x) => x; return); let mut new_stmts = place_here.iter() - .map(|&id| mk().local_stmt(&locals[&id].local)) + .map(|&id| { + let local = &locals[&id].local; + // Set the statement's span to match the local's span. + // + // By default, mk().local_stmt() creates a Stmt with DUMMY_SP. The rewrite system + // uses stmt.span (via the Splice trait) to extract source text for recovery during + // the print/reparse cycle. Without a valid span, source extraction fails. + // + // Setting stmt.span = local.span provides a valid source location. The Splice + // implementation in print.rs will extend this to cover any attributes. + mk().span(local.span).local_stmt(local) + }) .collect::>(); new_stmts.append(&mut b.stmts); b.stmts = new_stmts; @@ -415,7 +426,20 @@ impl Transform for FoldLetAssign { let local_mark = local_pos.remove(&hir_id).unwrap(); let mut l = local.clone(); l.kind = LocalKind::Init(init); - curs.replace(|_| mk().local_stmt(l)); + + // Preserve the original local's span when creating the combined statement. + // + // fold_let_assign transforms: + // let x; <- original local + // x = expr; <- assignment + // into: + // let x = expr; <- combined local + // + // The combined Local keeps l.span from the original declaration. Setting + // stmt.span = l.span allows the rewrite system to extract source text using + // the Splice trait. Without this, stmt.span would be DUMMY_SP and source + // extraction would fail. + curs.replace(move |_| mk().span(l.span).local_stmt(l)); let here = curs.mark(); curs.seek(local_mark); @@ -514,6 +538,7 @@ impl Transform for RemoveRedundantLetTypes { let pat = mcx.parse_stmts("let $pat:Pat : $ty:Ty = $init:Expr;"); let repl = mcx.parse_stmts("let $pat = $init;"); mut_visit_match_with(mcx, pat, krate, |ast, mcx| { + let orig_span = ast.get(0).map(|s| s.span); let e = mcx.bindings.get::<_, P>("$init").unwrap(); let e_ty = cx.adjusted_node_type(e.id); let e_ty = tcx.normalize_erasing_regions(ParamEnv::empty(), e_ty); @@ -523,6 +548,14 @@ impl Transform for RemoveRedundantLetTypes { let t_ty = tcx.normalize_erasing_regions(ParamEnv::empty(), t_ty); if e_ty == t_ty { *ast = repl.clone().subst(st, cx, &mcx.bindings); + // Preserve the original statement's span after replacement. + // + // The subst() call creates a new Stmt from the replacement pattern, which has + // a synthetic span from the pattern parse. Restoring the original span allows + // the rewrite system to extract source text via the Splice trait. + if let (Some(span), Some(first)) = (orig_span, ast.get_mut(0)) { + first.span = span; + } } }) } From 1504af6aaa92fde161edf5c03b0d6c5013d6ff85 Mon Sep 17 00:00:00 2001 From: David Anekstein Date: Fri, 21 Nov 2025 21:51:29 -0500 Subject: [PATCH 2/3] remove span expansion --- c2rust-refactor/src/rewrite/strategy/print.rs | 33 +++++-------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/c2rust-refactor/src/rewrite/strategy/print.rs b/c2rust-refactor/src/rewrite/strategy/print.rs index 495d040ef1..75d0836231 100644 --- a/c2rust-refactor/src/rewrite/strategy/print.rs +++ b/c2rust-refactor/src/rewrite/strategy/print.rs @@ -252,24 +252,7 @@ impl Splice for Ty { impl Splice for Stmt { fn splice_span(&self) -> Span { - // Extend statement span to include attributes on the inner node. - // - // Transforms like sink_lets set stmt.span = local.span, but attributes are - // stored on the inner Local/Item/Expr, not the Stmt wrapper. The pretty-printer - // outputs attributes, but if stmt.span doesn't cover them, the rewrite system - // can't extract the complete source text. - // - // extend_span_attrs() expands the span backward to include attributes that - // appear before the node in source, checking syntax contexts to avoid crossing - // macro boundaries. - let attrs = match &self.kind { - StmtKind::Local(local) => &local.attrs[..], - StmtKind::Item(item) => &item.attrs[..], - StmtKind::Expr(expr) | StmtKind::Semi(expr) => &expr.attrs[..], - StmtKind::MacCall(mac) => &mac.attrs[..], - StmtKind::Empty => &[], - }; - extend_span_attrs(self.span, attrs) + self.span } } @@ -695,13 +678,15 @@ where // Fallback: extract source snippet when pretty-printing produces empty output. // - // When transforms move macro-expanded code (e.g., from #[derive]), the span may - // point to generated code with no source file location. Pretty-printing such nodes - // can produce empty output, which causes problems during the reparse/recovery phase. + // The rustc pretty-printer (pprust) produces empty output for AST nodes with + // DUMMY_NODE_ID. This commonly occurs when transforms like sink_lets create new + // statement wrappers with DUMMY_NODE_ID to distinguish transformed nodes from + // originals (see vars.rs:79). Without this fallback, empty pretty-printer output + // causes parsing failures (0 statements instead of 1), crashing the rewrite system. // - // If the printed text is empty, try extracting the source directly using - // splice_span() (which includes attributes). If source is available, use that - // for reparsing instead of the empty pretty-printed text. + // The fallback extracts source text directly from the node's span using + // span_to_snippet(). This requires that statements have valid spans set, which + // is why both span preservation and this fallback mechanism are necessary. if printed.trim().is_empty() { if let Ok(snippet) = rcx .session() From 8355dba367af1785aa73f4465d7ee3e6809e1e6c Mon Sep 17 00:00:00 2001 From: David Anekstein Date: Fri, 21 Nov 2025 22:08:34 -0500 Subject: [PATCH 3/3] fix comment --- c2rust-refactor/src/rewrite/strategy/print.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/c2rust-refactor/src/rewrite/strategy/print.rs b/c2rust-refactor/src/rewrite/strategy/print.rs index 75d0836231..7bda3ada86 100644 --- a/c2rust-refactor/src/rewrite/strategy/print.rs +++ b/c2rust-refactor/src/rewrite/strategy/print.rs @@ -678,15 +678,11 @@ where // Fallback: extract source snippet when pretty-printing produces empty output. // - // The rustc pretty-printer (pprust) produces empty output for AST nodes with - // DUMMY_NODE_ID. This commonly occurs when transforms like sink_lets create new - // statement wrappers with DUMMY_NODE_ID to distinguish transformed nodes from - // originals (see vars.rs:79). Without this fallback, empty pretty-printer output - // causes parsing failures (0 statements instead of 1), crashing the rewrite system. - // - // The fallback extracts source text directly from the node's span using - // span_to_snippet(). This requires that statements have valid spans set, which - // is why both span preservation and this fallback mechanism are necessary. + // pprust can emit "" for Local statements whose inner Local has DUMMY_NODE_ID + // (sink_lets sets that intentionally). Reparse of "" yields 0 statements and + // hits the lone() assertion. The fallback replaces the empty string with the + // original source via span_to_snippet(), assuming the statement carries a real + // span (set in vars.rs). if printed.trim().is_empty() { if let Ok(snippet) = rcx .session()