refactor: avoid reordering attributes when restoring them

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<Attribute>, 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<Attribute>, 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 {

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 {}

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 {}

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