diff --git a/c2rust-refactor/src/transform/reorganize_definitions.rs b/c2rust-refactor/src/transform/reorganize_definitions.rs index 4acabe34fc..10526666f1 100644 --- a/c2rust-refactor/src/transform/reorganize_definitions.rs +++ b/c2rust-refactor/src/transform/reorganize_definitions.rs @@ -72,6 +72,14 @@ pub struct Reorganizer<'a, 'tcx: 'a> { // Counter used by `unique_ident` ident_counter: HashMap, + + // Definitions that `match_defs` redirected a header declaration to, and + // which are not `pub`. `is_exported` accepts a `#[no_mangle]` value + // whatever its Rust visibility, because that is what decides whether the + // C symbol is visible to another translation unit; but the paths that now + // point at the definition come from other modules, which a private item + // cannot be named from. `widen_matched_defs` widens these. + widened_defs: HashSet, } #[derive(Clone)] @@ -124,6 +132,7 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> { bitfield_ty_defs: HashMap::new(), stdlib_id: DUMMY_NODE_ID, ident_counter: HashMap::new(), + widened_defs: HashSet::new(), } } @@ -135,6 +144,7 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> { let mut header_decls = self.remove_header_items(krate); self.match_defs(&mut header_decls, krate); + self.widen_matched_defs(krate); self.update_module_info_items(krate); self.move_items(header_decls, krate); @@ -518,6 +528,9 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> { DeclKind::ForeignItem(foreign, _) => foreign_equiv(&foreign, item), }); if !decl_ids.is_empty() { + if !item.vis.kind.is_pub() { + self.widened_defs.insert(item.id); + } let def_id = self.cx.node_def_id(item.id); let dest_path = self.cx.def_path(def_id); let ldid = def_id.expect_local(); @@ -680,6 +693,32 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> { } } + /// Widen the visibility of the definitions `match_defs` redirected header + /// declarations to, so that the rewritten paths can name them. + /// + /// `match_defs` accepts a definition that `is_exported` calls exported, + /// which for a value means it carries `#[no_mangle]` or `#[export_name]` — + /// a statement about the C symbol, not about Rust visibility. Such a + /// definition can be private to its module while the declarations it + /// replaces were used from other modules, whose paths now point at it. + fn widen_matched_defs(&self, krate: &mut Crate) { + if self.widened_defs.is_empty() { + return; + } + // The users are all in this crate, so `pub(crate)` is enough. + let crate_vis = VisibilityKind::Restricted { + path: P(Path::from_ident(Ident::new(kw::Crate, DUMMY_SP))), + id: DUMMY_NODE_ID, + shorthand: true, + }; + FlatMapNodes::visit(krate, |mut item: P| { + if self.widened_defs.contains(&item.id) { + item.vis.kind = join_visibility(&item.vis.kind, &crate_vis); + } + smallvec![item] + }); + } + /// Update items set in ModuleInfos with current remaining items in that /// module so that we don't override an existing item fn update_module_info_items(&mut self, krate: &Crate) { diff --git a/c2rust-refactor/tests/snapshots.rs b/c2rust-refactor/tests/snapshots.rs index 9db57a2925..630b2f0221 100644 --- a/c2rust-refactor/tests/snapshots.rs +++ b/c2rust-refactor/tests/snapshots.rs @@ -440,9 +440,7 @@ fn test_reorder_derives() { #[cfg(target_os = "linux")] // `statvfs` and `statfs64` are Linux only. #[test] fn test_reorganize_definitions() { - refactor("reorganize_definitions") - .new_expect_compile_error(true) - .test(); + refactor("reorganize_definitions").test(); } #[test] @@ -558,6 +556,16 @@ fn test_reorganize_non_ascii_ident() { .test(); } +/// A definition that a header declaration is matched to has to be nameable +/// from the modules whose paths are rewritten to point at it, even when it +/// is only "exported" in the sense of carrying `#[no_mangle]`. +#[test] +fn test_reorganize_private_no_mangle_def() { + refactor("reorganize_definitions") + .named("reorganize_private_no_mangle_def.rs") + .test(); +} + #[test] fn test_reorganize_self_import_destination() { refactor("reorganize_definitions") diff --git a/c2rust-refactor/tests/snapshots/reorganize_private_no_mangle_def.rs b/c2rust-refactor/tests/snapshots/reorganize_private_no_mangle_def.rs new file mode 100644 index 0000000000..c940683f93 --- /dev/null +++ b/c2rust-refactor/tests/snapshots/reorganize_private_no_mangle_def.rs @@ -0,0 +1,43 @@ +#![feature(rustc_private)] +#![feature(register_tool)] +#![register_tool(c2rust)] +#![allow(non_upper_case_globals)] +#![allow(non_camel_case_types)] +#![allow(dead_code)] + +extern crate libc; + +// `counter` carries `#[no_mangle]`, so it is visible to the linker from +// another translation unit, but it is private to its module in Rust terms. +// The header declaration in `user` is matched to it and every path to the +// declaration is rewritten to point here, from outside this module. + +pub mod def { + use libc; + + #[no_mangle] + #[c2rust::src_loc = "10:0"] + static mut counter: libc::c_int = 0; +} + +pub mod user { + use libc; + + #[c2rust::header_src = "/home/user/some/workspace/counter.h:1"] + pub mod counter_h { + use super::libc; + + extern "C" { + #[c2rust::src_loc = "2:0"] + pub static mut counter: libc::c_int; + } + } + + use counter_h::counter; + + pub unsafe fn read() -> libc::c_int { + counter + } +} + +fn main() {} diff --git a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_private_no_mangle_def.rs.snap b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_private_no_mangle_def.rs.snap new file mode 100644 index 0000000000..fef10c7825 --- /dev/null +++ b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_private_no_mangle_def.rs.snap @@ -0,0 +1,40 @@ +--- +source: c2rust-refactor/tests/snapshots.rs +expression: c2rust-refactor reorganize_definitions --rewrite-mode alongside -- tests/snapshots/reorganize_private_no_mangle_def.rs --edition 2021 +--- +#![feature(rustc_private)] +#![feature(register_tool)] +#![register_tool(c2rust)] +#![allow(non_upper_case_globals)] +#![allow(non_camel_case_types)] +#![allow(dead_code)] + +pub mod counter_h { + use ::libc; +} +extern crate libc; + +// `counter` carries `#[no_mangle]`, so it is visible to the linker from +// another translation unit, but it is private to its module in Rust terms. +// The header declaration in `user` is matched to it and every path to the +// declaration is rewritten to point here, from outside this module. + +pub mod def { + use libc; + + #[no_mangle] + + pub(crate) static mut counter: libc::c_int = 0; +} + +pub mod user { + use libc; + + use crate::def::counter; + + pub unsafe fn read() -> libc::c_int { + crate::def::counter + } +} + +fn main() {} diff --git a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions.rs.snap b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions.rs.snap index 0914695284..ac996c745e 100644 --- a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions.rs.snap +++ b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions.rs.snap @@ -96,7 +96,7 @@ pub mod bar { type FooInt = i32; #[no_mangle] - static mut Bar: crate::bar::bar_t = crate::bar::bar_t { + pub(crate) static mut Bar: crate::bar::bar_t = crate::bar::bar_t { alloc: 0 as *mut libc::c_char, data: 0 as *mut libc::c_char, i: 0,