diff --git a/c2rust-refactor/src/transform/reorganize_definitions.rs b/c2rust-refactor/src/transform/reorganize_definitions.rs index 2cd7537cb0..6de7fde65f 100644 --- a/c2rust-refactor/src/transform/reorganize_definitions.rs +++ b/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 diff --git a/c2rust-refactor/tests/snapshots.rs b/c2rust-refactor/tests/snapshots.rs index bb5c947169..f4a5738beb 100644 --- a/c2rust-refactor/tests/snapshots.rs +++ b/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") diff --git a/c2rust-refactor/tests/snapshots/reorganize_implicit_extern.rs b/c2rust-refactor/tests/snapshots/reorganize_implicit_extern.rs new file mode 100644 index 0000000000..0a89d2d7be --- /dev/null +++ b/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() {} diff --git a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_foreign_fn_arity.rs.snap b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_foreign_fn_arity.rs.snap index 89fb446471..b394fee6e0 100644 --- a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_foreign_fn_arity.rs.snap +++ b/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; diff --git a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_foreign_fn_type_identity.rs.snap b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_foreign_fn_type_identity.rs.snap index 65ea8feff6..ae7255eb1f 100644 --- a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_foreign_fn_type_identity.rs.snap +++ b/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)] diff --git a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_foreign_item_rename.rs.snap b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_foreign_item_rename.rs.snap index 945ea521a4..717a04045a 100644 --- a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_foreign_item_rename.rs.snap +++ b/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; diff --git a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_implicit_extern.rs.snap b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_implicit_extern.rs.snap new file mode 100644 index 0000000000..bffcefa4bc --- /dev/null +++ b/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() {}