refactor: fix nondeterministic ordering for orphaned impls

c2rust-refactor/src/transform/reorganize_definitions.rs

@@ -376,7 +376,10 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> {
         // individually processed as a moved declaration — is deferred to
         // `pending_impls` and reattached directly to the self type's
         // destination module in `move_items`, once that is known.
-        let mut impls: HashMap<DefId, MovedDeclImpl> = HashMap::new();
+        // An `IndexMap` because the entries left over below are appended to
+        // `pending_impls` by iterating it, and `move_items` emits those in
+        // order; `HashMap` iteration order varies between runs.
+        let mut impls: IndexMap<DefId, MovedDeclImpl> = IndexMap::new();
         let mut pending_impls: Vec<(DefId, MovedDeclImpl)> = Vec::new();
         FlatMapNodes::visit(krate, |mut item: P<Item>| {
             if let Some((path, _)) = parse_source_header(&item.attrs) {
@@ -431,13 +434,8 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> {
                             item: item.clone(),
                             parent_header: HeaderInfo::new(header_ident, path.clone()),
                         };
-                        if impl_is_carryable(r#impl) {
-                            match impls.entry(decl_def_id) {
-                                Entry::Vacant(entry) => {
-                                    entry.insert(moved_impl);
-                                }
-                                Entry::Occupied(_) => pending_impls.push((decl_def_id, moved_impl)),
-                            }
+                        if impl_is_carryable(r#impl) && !impls.contains_key(&decl_def_id) {
+                            impls.insert(decl_def_id, moved_impl);
                         } else {
                             pending_impls.push((decl_def_id, moved_impl));
                         }
@@ -517,7 +515,7 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> {
                         let inserted = declarations.insert_item(
                             item.clone(),
                             header_info,
-                            impls.remove(&new_def_id),
+                            impls.shift_remove(&new_def_id),
                         );
                         // Keep the item if we are not collapsing it
                         !inserted

c2rust-refactor/tests/snapshots.rs

@@ -587,6 +587,16 @@ fn test_reorganize_non_ascii_ident() {
         .test();
 }
 
+/// `impl` blocks deferred to `pending_impls` must be reattached to their
+/// destination module in a stable order, so the output is the same from run
+/// to run.
+#[test]
+fn test_reorganize_orphaned_impl_order() {
+    refactor("reorganize_definitions")
+        .named("reorganize_orphaned_impl_order.rs")
+        .test();
+}
+
 /// `impl` blocks in header modules whose self type is defined outside the
 /// headers, or which contain a non-`const` item, must be reattached to
 /// their self type's destination module rather than dropped.

c2rust-refactor/tests/snapshots/reorganize_orphaned_impl_order.rs

@@ -0,0 +1,66 @@
+#![feature(register_tool)]
+#![register_tool(c2rust)]
+#![allow(non_upper_case_globals)]
+#![allow(non_camel_case_types)]
+#![allow(dead_code)]
+
+pub mod stuff {
+    // The types the header impls attach to are all defined here, outside any
+    // header module, so no moved declaration ever consumes their saved
+    // `impls` entries.
+    #[derive(Copy, Clone)]
+    #[repr(C)]
+    pub struct alpha {
+        pub x: i32,
+    }
+
+    #[derive(Copy, Clone)]
+    #[repr(C)]
+    pub struct beta {
+        pub y: i32,
+    }
+
+    #[derive(Copy, Clone)]
+    #[repr(C)]
+    pub struct gamma {
+        pub z: i32,
+    }
+}
+
+pub mod opts {
+    #[c2rust::header_src = "/home/user/some/workspace/opts.h:1"]
+    pub mod opts_h {
+        pub use crate::stuff::alpha;
+        pub use crate::stuff::beta;
+        pub use crate::stuff::gamma;
+
+        // Three const-only impls, each on a different self type. All three
+        // qualify to be carried alongside a moved declaration, so all three
+        // land in the `impls` map, and none of them is ever claimed. They
+        // must be reattached to `stuff` in a stable order.
+        #[c2rust::src_loc = "2:0"]
+        impl alpha {
+            #[c2rust::src_loc = "3:0"]
+            pub const DEFAULT: i32 = 1;
+        }
+
+        #[c2rust::src_loc = "4:0"]
+        impl beta {
+            #[c2rust::src_loc = "5:0"]
+            pub const DEFAULT: i32 = 2;
+        }
+
+        #[c2rust::src_loc = "6:0"]
+        impl gamma {
+            #[c2rust::src_loc = "7:0"]
+            pub const DEFAULT: i32 = 3;
+        }
+    }
+    use self::opts_h::{alpha, beta, gamma};
+
+    pub fn go() -> i32 {
+        alpha::DEFAULT + beta::DEFAULT + gamma::DEFAULT
+    }
+}
+
+fn main() {}

c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_orphaned_impl_order.rs.snap

@@ -0,0 +1,62 @@
+---
+source: c2rust-refactor/tests/snapshots.rs
+expression: c2rust-refactor reorganize_definitions --rewrite-mode alongside -- tests/snapshots/reorganize_orphaned_impl_order.rs --edition 2021
+---
+#![feature(register_tool)]
+#![register_tool(c2rust)]
+#![allow(non_upper_case_globals)]
+#![allow(non_camel_case_types)]
+#![allow(dead_code)]
+
+pub mod stuff {
+    // The types the header impls attach to are all defined here, outside any
+    // header module, so no moved declaration ever consumes their saved
+    // `impls` entries.
+    #[derive(Copy, Clone)]
+    #[repr(C)]
+    pub struct alpha {
+        pub x: i32,
+    }
+
+    #[derive(Copy, Clone)]
+    #[repr(C)]
+    pub struct beta {
+        pub y: i32,
+    }
+
+    #[derive(Copy, Clone)]
+    #[repr(C)]
+    pub struct gamma {
+        pub z: i32,
+    }
+    // Three const-only impls, each on a different self type. All three
+    // qualify to be carried alongside a moved declaration, so all three
+    // land in the `impls` map, and none of them is ever claimed. They
+    // must be reattached to `stuff` in a stable order.
+
+    impl alpha {
+        pub const DEFAULT: i32 = 1;
+    }
+
+    impl beta {
+        pub const DEFAULT: i32 = 2;
+    }
+
+    impl gamma {
+        pub const DEFAULT: i32 = 3;
+    }
+}
+
+pub mod opts {
+
+    // =============== BEGIN opts_h ================
+    pub use crate::stuff::alpha;
+    pub use crate::stuff::beta;
+    pub use crate::stuff::gamma;
+
+    pub fn go() -> i32 {
+        alpha::DEFAULT + beta::DEFAULT + gamma::DEFAULT
+    }
+}
+
+fn main() {}