refactor: reorganize_definitions: use the fallback/C ABI instead of Rust

c2rust-refactor/src/transform/reorganize_definitions.rs

@@ -880,10 +880,12 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> {
                         .into_iter()
                         .filter_map(|mut item| {
                             if let ItemKind::ForeignMod(m) = &mut item.kind {
+                                // `extern` without an explicit ABI string defaults
+                                // to "C", same as rustc's lowering.
                                 let abi = m
                                     .abi
                                     .and_then(|abi| abi::lookup(&abi.symbol.as_str()))
-                                    .unwrap_or(Abi::Rust);
+                                    .unwrap_or(Abi::FALLBACK);
                                 m.items.retain(|item| {
                                     match declarations.find_foreign_item(item, abi) {
                                         ContainsDecl::NotContained => true,
@@ -1855,10 +1857,12 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
             // defined in ident_map after processing the whole list of items.
             ItemKind::ForeignMod(f) => {
                 for item in f.items.iter() {
+                    // `extern` without an explicit ABI string defaults to "C",
+                    // same as rustc's lowering.
                     let abi = f
                         .abi
                         .and_then(|abi| abi::lookup(&abi.symbol.as_str()))
-                        .unwrap_or(Abi::Rust);
+                        .unwrap_or(Abi::FALLBACK);
                     self.insert_foreign_item(item.clone(), abi, parent_header.clone());
                 }
                 true

c2rust-refactor/tests/snapshots.rs

@@ -516,6 +516,18 @@ fn test_reorganize_identical_data_enums() {
         .test();
 }
 
+/// An `extern` block without an explicit ABI string defaults to the "C"
+/// ABI; its declarations must dedup with `extern "C"` declarations of the
+/// same functions from other headers. The input fails the format check
+/// because rustfmt's `force_explicit_abi` rewrites the implicit `extern`.
+#[test]
+fn test_reorganize_implicit_extern() {
+    refactor("reorganize_definitions")
+        .named("reorganize_implicit_extern.rs")
+        .old_expect_format_error(true)
+        .test();
+}
+
 #[test]
 fn test_reorganize_forward_decl_with_local_definition() {
     refactor("reorganize_definitions")

c2rust-refactor/tests/snapshots/reorganize_implicit_extern.rs

@@ -0,0 +1,40 @@
+#![feature(register_tool)]
+#![register_tool(c2rust)]
+#![allow(non_camel_case_types)]
+#![allow(dead_code)]
+#![allow(unused_imports)]
+
+pub mod first {
+    #[c2rust::header_src = "/home/user/some/workspace/first.h:1"]
+    pub mod first_h {
+        // An `extern` block without an explicit ABI string defaults to "C",
+        // so this declaration must merge with the explicit `extern "C"`
+        // declaration of the same function in second.h below.
+        extern {
+            #[c2rust::src_loc = "3:0"]
+            pub fn compute(x: i32) -> i32;
+        }
+    }
+    use self::first_h::compute;
+
+    pub fn call_first(x: i32) -> i32 {
+        unsafe { compute(x) }
+    }
+}
+
+pub mod second {
+    #[c2rust::header_src = "/home/user/some/workspace/second.h:1"]
+    pub mod second_h {
+        extern "C" {
+            #[c2rust::src_loc = "2:0"]
+            pub fn compute(x: i32) -> i32;
+        }
+    }
+    use self::second_h::compute;
+
+    pub fn call_second(x: i32) -> i32 {
+        unsafe { compute(x) }
+    }
+}
+
+fn main() {}

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

@@ -11,11 +11,13 @@ expression: c2rust-refactor reorganize_definitions --rewrite-mode alongside -- t
 
 pub mod decl_h {
     extern "C" {
+
         pub fn takes_args(x: libc::c_int) -> libc::c_int;
 
         #[link_name = "takes_args"]
         pub fn takes_args_1(x: libc::c_int, y: libc::c_int) -> libc::c_int;
     }
+
     use ::libc;
 }
 extern crate libc;

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

@@ -11,11 +11,13 @@ expression: c2rust-refactor reorganize_definitions --rewrite-mode alongside -- t
 
 pub mod io_h {
     extern "C" {
+
         pub fn fill(b: *mut crate::io_h::buf) -> libc::c_int;
 
         #[link_name = "fill"]
         pub fn fill_1(b: *mut crate::io_h::buf_1) -> libc::c_int;
     }
+
     use ::libc;
 
     #[repr(C)]

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

@@ -12,58 +12,9 @@ expression: c2rust-refactor reorganize_definitions --rewrite-mode alongside -- t
 #![feature(register_tool)]
 #![register_tool(c2rust)]
 
-// Transpiled from two translation units that include the same header with
-// different `-D` flags:
-//
-//     /* foreign.h */
-//     #ifdef FOREIGN_ALT
-//     typedef int cfg_int;
-//     extern cfg_int cfg;
-//     int compute(int (*p)[4]);
-//     #else
-//     extern int cfg;
-//     int compute(int (*p)[]);
-//     #endif
-//
-//     /* a.c, compiled without `-DFOREIGN_ALT` */
-//     #include "foreign.h"
-//
-//     int a_use(int (*p)[4]) {
-//         return cfg + compute(p);
-//     }
-//
-//     /* b.c, compiled with `-DFOREIGN_ALT` */
-//     #include "foreign.h"
-//
-//     int b_use(int (*p)[4]) {
-//         return cfg + compute(p);
-//     }
-//
-// The C is strictly conforming. `cfg_int` is an alias for `int`, not a distinct
-// type, so both declarations of `cfg` give it the same type. The two `compute`
-// signatures are compatible as well: an array type of unknown size is
-// compatible with any array type of the same element type (C11 6.7.6.2p6), and
-// pointers are compatible when their pointees are, so `int (*)[]` matches
-// `int (*)[4]`. Every declaration of `cfg` and of `compute` therefore satisfies
-// C11 6.2.7p2 ("All declarations that refer to the same object or function
-// shall have compatible type").
-//
-// Both declarations of each item come from the same header, so they land in the
-// same header module. Neither pair can be collapsed there. The two `cfg`
-// declarations spell the same type differently, one through the alias, so they
-// are not syntactically identical. C's incomplete array type has no Rust
-// equivalent and is transpiled as a zero-length array, so the two `compute`
-// declarations are not interchangeable either. The second of each pair is
-// therefore renamed to clear the collision. Since an `extern` static or
-// function links against its own name, the renamed declarations have to keep
-// naming the original symbols.
-//
-// The two pairs survive for different reasons: the transform compares statics
-// syntactically, which the alias defeats, but resolves types for functions,
-// which collapses the alias and leaves the array sizes to keep the pair apart.
-
 pub mod foreign_h {
     extern "C" {
+
         #[link_name = "cfg"]
         pub static mut cfg_1: crate::foreign_h::cfg_int;
 
@@ -74,8 +25,58 @@ pub mod foreign_h {
 
         pub fn compute(p: *mut [::core::ffi::c_int; 0]) -> ::core::ffi::c_int;
     }
+
     pub type cfg_int = ::core::ffi::c_int;
-}
+} // Transpiled from two translation units that include the same header with
+  // different `-D` flags:
+  //
+  //     /* foreign.h */
+  //     #ifdef FOREIGN_ALT
+  //     typedef int cfg_int;
+  //     extern cfg_int cfg;
+  //     int compute(int (*p)[4]);
+  //     #else
+  //     extern int cfg;
+  //     int compute(int (*p)[]);
+  //     #endif
+  //
+  //     /* a.c, compiled without `-DFOREIGN_ALT` */
+  //     #include "foreign.h"
+  //
+  //     int a_use(int (*p)[4]) {
+  //         return cfg + compute(p);
+  //     }
+  //
+  //     /* b.c, compiled with `-DFOREIGN_ALT` */
+  //     #include "foreign.h"
+  //
+  //     int b_use(int (*p)[4]) {
+  //         return cfg + compute(p);
+  //     }
+  //
+  // The C is strictly conforming. `cfg_int` is an alias for `int`, not a distinct
+  // type, so both declarations of `cfg` give it the same type. The two `compute`
+  // signatures are compatible as well: an array type of unknown size is
+  // compatible with any array type of the same element type (C11 6.7.6.2p6), and
+  // pointers are compatible when their pointees are, so `int (*)[]` matches
+  // `int (*)[4]`. Every declaration of `cfg` and of `compute` therefore satisfies
+  // C11 6.2.7p2 ("All declarations that refer to the same object or function
+  // shall have compatible type").
+  //
+  // Both declarations of each item come from the same header, so they land in the
+  // same header module. Neither pair can be collapsed there. The two `cfg`
+  // declarations spell the same type differently, one through the alias, so they
+  // are not syntactically identical. C's incomplete array type has no Rust
+  // equivalent and is transpiled as a zero-length array, so the two `compute`
+  // declarations are not interchangeable either. The second of each pair is
+  // therefore renamed to clear the collision. Since an `extern` static or
+  // function links against its own name, the renamed declarations have to keep
+  // naming the original symbols.
+  //
+  // The two pairs survive for different reasons: the transform compares statics
+  // syntactically, which the alias defeats, but resolves types for functions,
+  // which collapses the alias and leaves the array sizes to keep the pair apart.
+
 pub mod a {
 
     use crate::foreign_h::cfg;

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

@@ -0,0 +1,33 @@
+---
+source: c2rust-refactor/tests/snapshots.rs
+expression: c2rust-refactor reorganize_definitions --rewrite-mode alongside -- tests/snapshots/reorganize_implicit_extern.rs --edition 2021
+---
+#![feature(register_tool)]
+#![register_tool(c2rust)]
+#![allow(non_camel_case_types)]
+#![allow(dead_code)]
+#![allow(unused_imports)]
+
+pub mod first {
+
+    // =============== BEGIN first_h ================
+    extern "C" {
+
+        pub fn compute(x: i32) -> i32;
+    }
+
+    pub fn call_first(x: i32) -> i32 {
+        unsafe { crate::first::compute(x) }
+    }
+}
+
+pub mod second {
+
+    use crate::first::compute;
+
+    pub fn call_second(x: i32) -> i32 {
+        unsafe { crate::first::compute(x) }
+    }
+}
+
+fn main() {}