Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions c2rust-refactor/src/rewrite/strategy/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're fixing sink_lets below, is this also needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that the attributes need to sink as well, without this the extracted source code span would just be the let statement but with this we cover the attribute as well and so we'll get a match to the pretty printed code (which includes the attribute)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We create a bunch of new mk()...local_stmt(...) below, couldn't we copy the attributes there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not exactly sure how that would be done, can you put attributes inside a statement? I thought that was contained at a higher level.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can put attributes on a statement, wouldn't that work here?

// 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)
}
}

Expand Down Expand Up @@ -674,7 +691,29 @@ fn rewrite_at_impl<T>(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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this reinsert back code that the transforms decided to delete?

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();

Expand Down
37 changes: 35 additions & 2 deletions c2rust-refactor/src/transform/vars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>();
new_stmts.append(&mut b.stmts);
b.stmts = new_stmts;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<Expr>>("$init").unwrap();
let e_ty = cx.adjusted_node_type(e.id);
let e_ty = tcx.normalize_erasing_regions(ParamEnv::empty(), e_ty);
Expand All @@ -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;
}
}
})
}
Expand Down