diff --git a/c2rust-refactor/src/collapse/macros.rs b/c2rust-refactor/src/collapse/macros.rs index d475358466..0c3f8e0ec5 100644 --- a/c2rust-refactor/src/collapse/macros.rs +++ b/c2rust-refactor/src/collapse/macros.rs @@ -338,18 +338,20 @@ impl<'a> MutVisitor for CollapseMacros<'a> { /// with known effects (such as `#[cfg]`, which removes itself when the condition is met) and tries /// to reverse those specific effects on `new.attrs`. fn restore_attrs(new_attrs: &mut Vec, old_attrs: &[Attribute]) { - // If the original item had a `#[derive]` attr, transfer it to the new one. - // TODO: handle multiple instances of `#[derive]` - // TODO: try to keep attrs in the same order - if let Some(attr) = crate::util::find_by_name(old_attrs, sym::derive) { - if !crate::util::contains_name(&new_attrs, sym::derive) { - new_attrs.push(attr.clone()); - } + fn same_attr(a: &Attribute, b: &Attribute) -> bool { + Iterator::eq( + a.tokens().to_tokenstream().into_trees(), + b.tokens().to_tokenstream().into_trees(), + ) } - if let Some(attr) = crate::util::find_by_name(old_attrs, sym::cfg) { - if !crate::util::contains_name(&new_attrs, sym::cfg) { - new_attrs.push(attr.clone()); + // If the original item had `#[derive]` or `#[cfg]` attrs, transfer them to the new one. + for old_attr in old_attrs.iter().rev() { + if old_attr.has_name(sym::derive) || old_attr.has_name(sym::cfg) { + // Only copy the attr if an identical one is not already present. + if !new_attrs.iter().any(|a| same_attr(a, old_attr)) { + new_attrs.push(old_attr.clone()); + } } } @@ -358,6 +360,9 @@ fn restore_attrs(new_attrs: &mut Vec, old_attrs: &[Attribute]) { // (It can be written explicitly, but is also inserted by #[derive(Eq)].) !attr.has_name(sym::structural_match) }); + + // Sort the attributes by their original source position to avoid reordering. + new_attrs.sort_by_key(|attr| attr.span.lo()); } fn spans_overlap(sp1: Span, sp2: Span) -> bool { diff --git a/c2rust-refactor/tests/reorder_derives/new.rs b/c2rust-refactor/tests/reorder_derives/new.rs new file mode 100644 index 0000000000..ade9a2c1cd --- /dev/null +++ b/c2rust-refactor/tests/reorder_derives/new.rs @@ -0,0 +1,26 @@ +#![allow( + dead_code, + non_camel_case_types, + non_snake_case, + non_upper_case_globals, + unused_assignments, + unused_mut +)] + +#[derive(Copy, Clone)] +#[repr(C)] +pub struct S0 {} + +#[derive(Clone)] +#[repr(u8)] +#[derive(Copy)] +#[derive(Debug)] +pub struct S1 {} + +#[cfg(not(test))] +#[derive(Debug, Copy, Clone)] +#[repr(u32)] +pub struct S2 {} + +#[derive(Debug)] +pub struct S3 {} diff --git a/c2rust-refactor/tests/reorder_derives/old.rs b/c2rust-refactor/tests/reorder_derives/old.rs new file mode 100644 index 0000000000..ade9a2c1cd --- /dev/null +++ b/c2rust-refactor/tests/reorder_derives/old.rs @@ -0,0 +1,26 @@ +#![allow( + dead_code, + non_camel_case_types, + non_snake_case, + non_upper_case_globals, + unused_assignments, + unused_mut +)] + +#[derive(Copy, Clone)] +#[repr(C)] +pub struct S0 {} + +#[derive(Clone)] +#[repr(u8)] +#[derive(Copy)] +#[derive(Debug)] +pub struct S1 {} + +#[cfg(not(test))] +#[derive(Debug, Copy, Clone)] +#[repr(u32)] +pub struct S2 {} + +#[derive(Debug)] +pub struct S3 {} diff --git a/c2rust-refactor/tests/reorder_derives/run.sh b/c2rust-refactor/tests/reorder_derives/run.sh new file mode 100755 index 0000000000..17f1cfadfe --- /dev/null +++ b/c2rust-refactor/tests/reorder_derives/run.sh @@ -0,0 +1,10 @@ +#!/bin/sh + +# work around System Integrity Protection on macOS +if [ `uname` = 'Darwin' ]; then + export LD_LIBRARY_PATH=$not_LD_LIBRARY_PATH +fi + +$refactor \ + test_one_plus_one \ + -- old.rs $rustflags