diff --git a/c2rust-refactor/src/rewrite/base.rs b/c2rust-refactor/src/rewrite/base.rs index db98cbaa8f..a072aa7da7 100644 --- a/c2rust-refactor/src/rewrite/base.rs +++ b/c2rust-refactor/src/rewrite/base.rs @@ -348,18 +348,36 @@ where // There's an item on the right corresponding to nothing on the left. // Insert the item before the current item on the left, rewriting // recursively. + // + // Rewrites and deletions of the neighboring old items claim + // their comment-extended spans, so anchor the insertion + // outside those extended spans; an insertion inside one + // would conflict with it. Inserting exactly at the extended + // start of the next item is fine: `cleanup_rewrites` orders + // the insertion before the following rewrite. + let extended_span = |idx: usize| { + let span = ast(&old[idx]).splice_span(); + match old_ids[idx] { + SeqItemId::Node(id) => extend_span_comments(&id, span, &rcx), + _ => span, + } + }; let before = if i > 0 { - ast(&old[i - 1]).splice_span() + extended_span(i - 1) } else { outer_span.shrink_to_lo() }; let after = if i < old.len() { - ast(&old[i]).splice_span() + extended_span(i) } else { outer_span.shrink_to_hi() }; - let old_span = if is_rewritable(before) { + let old_span = if i == 0 && i < old.len() && is_rewritable(after) { + // The outer span's start may fall inside the first item's + // comment-extended span; insert before the comments. + after.with_hi(after.lo()) + } else if is_rewritable(before) { before.with_lo(before.hi()) } else if is_rewritable(after) { after.with_hi(after.lo()) @@ -460,18 +478,30 @@ where // There's an item on the right corresponding to nothing on the left. // Insert the item before the current item on the left, rewriting // recursively. + // + // As in `rewrite_seq`, anchor the insertion outside the + // neighbors' comment-extended spans, which their own + // rewrites and deletions claim. + let extended_span = |idx: usize| match old_ids[idx] { + SeqItemId::Node(id) => extend_span_comments(&id, old_spans[idx], &rcx), + _ => old_spans[idx], + }; let before = if i > 0 { - old_spans[i - 1] + extended_span(i - 1) } else { outer_span.shrink_to_lo() }; let after = if i < old.len() { - old_spans[i] + extended_span(i) } else { outer_span.shrink_to_hi() }; - let old_span = if is_rewritable(before) { + let old_span = if i == 0 && i < old.len() && is_rewritable(after) { + // The outer span's start may fall inside the first item's + // comment-extended span; insert before the comments. + after.with_hi(after.lo()) + } else if is_rewritable(before) { before.with_lo(before.hi()) } else if is_rewritable(after) { after.with_hi(after.lo()) diff --git a/c2rust-refactor/tests/snapshots/reorganize_split_renamed_import.rs b/c2rust-refactor/tests/snapshots/reorganize_split_renamed_import.rs index 40aa7fda1a..53f37c06d4 100644 --- a/c2rust-refactor/tests/snapshots/reorganize_split_renamed_import.rs +++ b/c2rust-refactor/tests/snapshots/reorganize_split_renamed_import.rs @@ -44,11 +44,11 @@ pub mod dest { pub fn dest_fn() {} } -// The `user` module holds one simple import resolving in both the type and -// value namespaces. After the reorganization the two targets have different -// paths (`tick_1` vs `tick`), so a second import must be added for the value -// namespace: the retained import only covers the renamed type. pub mod user { + // One simple import resolving in both the type and value namespaces. + // After the reorganization the two targets have different paths + // (`tick_1` vs `tick`), so a second import must be added for the value + // namespace: the retained import only covers the renamed type. use crate::dest::dest_h::tick; pub fn make(x: i32) -> tick { diff --git a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_split_renamed_import.rs.snap b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_split_renamed_import.rs.snap index ab912cabba..c713dfa5dc 100644 --- a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_split_renamed_import.rs.snap +++ b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_split_renamed_import.rs.snap @@ -48,12 +48,15 @@ pub mod dest { pub fn dest_fn() {} } -// The `user` module holds one simple import resolving in both the type and -// value namespaces. After the reorganization the two targets have different -// paths (`tick_1` vs `tick`), so a second import must be added for the value -// namespace: the retained import only covers the renamed type. pub mod user { use crate::dest::tick; + // One simple import resolving in both the type and value namespaces. + + // After the reorganization the two targets have different paths + + // (`tick_1` vs `tick`), so a second import must be added for the value + + // namespace: the retained import only covers the renamed type. use crate::dest::tick_1; pub fn make(x: i32) -> crate::dest::tick_1 {