From ff2010868de2608e131544b061c1072039ad31f0 Mon Sep 17 00:00:00 2001 From: Andrei Homescu Date: Fri, 24 Jul 2026 12:58:54 -0700 Subject: [PATCH 1/2] refactor: add test for macro namespace classification --- c2rust-refactor/tests/snapshots.rs | 13 ++++++ .../snapshots/reorganize_macro_namespace.rs | 41 ++++++++++++++++++ ...nitions-reorganize_macro_namespace.rs.snap | 43 +++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 c2rust-refactor/tests/snapshots/reorganize_macro_namespace.rs create mode 100644 c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_macro_namespace.rs.snap diff --git a/c2rust-refactor/tests/snapshots.rs b/c2rust-refactor/tests/snapshots.rs index f4a5738beb..a8675787f3 100644 --- a/c2rust-refactor/tests/snapshots.rs +++ b/c2rust-refactor/tests/snapshots.rs @@ -535,6 +535,19 @@ fn test_reorganize_forward_decl_with_local_definition() { .test(); } +/// TODO Broken. +/// A macro and a struct sharing a name live in different namespaces and do +/// not collide, so the struct should still move into the module holding the +/// macro. `item_namespaces` classifies `MacroDef` as `TypeNS` instead of +/// `MacroNS`, so the transform sees a type-namespace collision and banishes +/// the struct to a new `thing_h` module. +#[test] +fn test_reorganize_macro_namespace() { + refactor("reorganize_definitions") + .named("reorganize_macro_namespace.rs") + .test(); +} + #[test] fn test_reorganize_multi_namespace() { refactor("reorganize_definitions") diff --git a/c2rust-refactor/tests/snapshots/reorganize_macro_namespace.rs b/c2rust-refactor/tests/snapshots/reorganize_macro_namespace.rs new file mode 100644 index 0000000000..073ebb7d48 --- /dev/null +++ b/c2rust-refactor/tests/snapshots/reorganize_macro_namespace.rs @@ -0,0 +1,41 @@ +#![feature(register_tool)] +#![register_tool(c2rust)] +#![allow(non_camel_case_types)] +#![allow(dead_code)] + +pub mod thing { + #[c2rust::header_src = "/home/user/some/workspace/thing.h:1"] + pub mod thing_h { + #[c2rust::src_loc = "2:0"] + #[derive(Copy, Clone)] + #[repr(C)] + pub struct point { + pub x: i32, + } + } + + // A macro sharing the `point` spelling with the struct in the header + // above. Macros live in the macro namespace, so the two names do not + // collide and the struct should still move into this module. But + // `item_namespaces` classifies every item it doesn't recognize as + // `TypeNS`, `MacroDef` included, so `update_module_info_items` records + // `point` as occupying this module's *type* namespace and + // `find_destination_id` rejects `thing` as a destination for the struct. + // + // Note there is deliberately no `use self::thing_h::point;` here: an + // import of the struct would land in `import_targets` and let the + // conflict check see that the two `point`s are the same definition, + // masking the misclassification. + macro_rules! point { + () => { + 0 + }; + } + + pub fn go() -> i32 { + let p = thing_h::point { x: point!() }; + p.x + } +} + +fn main() {} diff --git a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_macro_namespace.rs.snap b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_macro_namespace.rs.snap new file mode 100644 index 0000000000..3c54169dcb --- /dev/null +++ b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_macro_namespace.rs.snap @@ -0,0 +1,43 @@ +--- +source: c2rust-refactor/tests/snapshots.rs +expression: c2rust-refactor reorganize_definitions --rewrite-mode alongside -- tests/snapshots/reorganize_macro_namespace.rs --edition 2021 +--- +#![feature(register_tool)] +#![register_tool(c2rust)] +#![allow(non_camel_case_types)] +#![allow(dead_code)] + +pub mod thing_h { + #[derive(Copy, Clone)] + #[repr(C)] + pub struct point { + pub x: i32, + } +} +pub mod thing { + + // A macro sharing the `point` spelling with the struct in the header + // above. Macros live in the macro namespace, so the two names do not + // collide and the struct should still move into this module. But + // `item_namespaces` classifies every item it doesn't recognize as + // `TypeNS`, `MacroDef` included, so `update_module_info_items` records + // `point` as occupying this module's *type* namespace and + // `find_destination_id` rejects `thing` as a destination for the struct. + // + // Note there is deliberately no `use self::thing_h::point;` here: an + // import of the struct would land in `import_targets` and let the + // conflict check see that the two `point`s are the same definition, + // masking the misclassification. + macro_rules! point { + () => { + 0 + }; + } + + pub fn go() -> i32 { + let p = crate::thing_h::point { x: point!() }; + p.x + } +} + +fn main() {} From dd8a665abdada18013af440dd6076efe11cc588a Mon Sep 17 00:00:00 2001 From: Andrei Homescu Date: Fri, 24 Jul 2026 13:02:09 -0700 Subject: [PATCH 2/2] refactor: fix macros misclassified in the type namespace `_ => Namespace::TypeNS` arm lumps `MacroDef` (MacroNS) into TypeNS. Harmless for transpiled C, but wrong in general. --- c2rust-refactor/src/context.rs | 5 +++++ .../src/transform/reorganize_definitions.rs | 6 ++++++ c2rust-refactor/tests/snapshots.rs | 7 ++----- .../snapshots/reorganize_macro_namespace.rs | 9 ++++----- ...finitions-reorganize_macro_namespace.rs.snap | 17 ++++++++--------- 5 files changed, 25 insertions(+), 19 deletions(-) diff --git a/c2rust-refactor/src/context.rs b/c2rust-refactor/src/context.rs index e7758c6149..f1ae1eb4f2 100644 --- a/c2rust-refactor/src/context.rs +++ b/c2rust-refactor/src/context.rs @@ -944,6 +944,11 @@ impl<'a, 'tcx> RefactorCtxt<'a, 'tcx> { smallvec![Namespace::TypeNS, Namespace::ValueNS] } + // A macro shares no namespace with a type or a value of the same + // name, so it must not be reported as occupying `TypeNS`; doing so + // makes an unrelated type of the same name look like a collision. + ItemKind::MacroDef(..) => smallvec![Namespace::MacroNS], + _ => smallvec![Namespace::TypeNS], } } diff --git a/c2rust-refactor/src/transform/reorganize_definitions.rs b/c2rust-refactor/src/transform/reorganize_definitions.rs index 6de7fde65f..5ee98271ad 100644 --- a/c2rust-refactor/src/transform/reorganize_definitions.rs +++ b/c2rust-refactor/src/transform/reorganize_definitions.rs @@ -519,6 +519,12 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> { // Values ItemKind::Static(..) | ItemKind::Const(..) | ItemKind::Fn(..) => Namespace::ValueNS, + // Macros. Kept in sync with `item_namespaces`, which decides + // the namespaces the declarations searched below were filed + // under; classifying a macro here as a type would look for it + // among the type declarations and never find it. + ItemKind::MacroDef(..) => Namespace::MacroNS, + // Types _ => Namespace::TypeNS, }; diff --git a/c2rust-refactor/tests/snapshots.rs b/c2rust-refactor/tests/snapshots.rs index a8675787f3..0f85c1a24d 100644 --- a/c2rust-refactor/tests/snapshots.rs +++ b/c2rust-refactor/tests/snapshots.rs @@ -535,12 +535,9 @@ fn test_reorganize_forward_decl_with_local_definition() { .test(); } -/// TODO Broken. /// A macro and a struct sharing a name live in different namespaces and do -/// not collide, so the struct should still move into the module holding the -/// macro. `item_namespaces` classifies `MacroDef` as `TypeNS` instead of -/// `MacroNS`, so the transform sees a type-namespace collision and banishes -/// the struct to a new `thing_h` module. +/// not collide, so the struct must still move into the module holding the +/// macro rather than being banished to a new `thing_h` module. #[test] fn test_reorganize_macro_namespace() { refactor("reorganize_definitions") diff --git a/c2rust-refactor/tests/snapshots/reorganize_macro_namespace.rs b/c2rust-refactor/tests/snapshots/reorganize_macro_namespace.rs index 073ebb7d48..97bcdcc56e 100644 --- a/c2rust-refactor/tests/snapshots/reorganize_macro_namespace.rs +++ b/c2rust-refactor/tests/snapshots/reorganize_macro_namespace.rs @@ -16,11 +16,10 @@ pub mod thing { // A macro sharing the `point` spelling with the struct in the header // above. Macros live in the macro namespace, so the two names do not - // collide and the struct should still move into this module. But - // `item_namespaces` classifies every item it doesn't recognize as - // `TypeNS`, `MacroDef` included, so `update_module_info_items` records - // `point` as occupying this module's *type* namespace and - // `find_destination_id` rejects `thing` as a destination for the struct. + // collide and the struct still moves into this module. Classifying + // `MacroDef` as a type instead would make `update_module_info_items` + // record `point` as occupying this module's *type* namespace, and + // `find_destination_id` would reject `thing` as a destination. // // Note there is deliberately no `use self::thing_h::point;` here: an // import of the struct would land in `import_targets` and let the diff --git a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_macro_namespace.rs.snap b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_macro_namespace.rs.snap index 3c54169dcb..20b4f5af7c 100644 --- a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_macro_namespace.rs.snap +++ b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_macro_namespace.rs.snap @@ -7,22 +7,21 @@ expression: c2rust-refactor reorganize_definitions --rewrite-mode alongside -- t #![allow(non_camel_case_types)] #![allow(dead_code)] -pub mod thing_h { +pub mod thing { + + // =============== BEGIN thing_h ================ #[derive(Copy, Clone)] #[repr(C)] pub struct point { pub x: i32, } -} -pub mod thing { // A macro sharing the `point` spelling with the struct in the header // above. Macros live in the macro namespace, so the two names do not - // collide and the struct should still move into this module. But - // `item_namespaces` classifies every item it doesn't recognize as - // `TypeNS`, `MacroDef` included, so `update_module_info_items` records - // `point` as occupying this module's *type* namespace and - // `find_destination_id` rejects `thing` as a destination for the struct. + // collide and the struct still moves into this module. Classifying + // `MacroDef` as a type instead would make `update_module_info_items` + // record `point` as occupying this module's *type* namespace, and + // `find_destination_id` would reject `thing` as a destination. // // Note there is deliberately no `use self::thing_h::point;` here: an // import of the struct would land in `import_targets` and let the @@ -35,7 +34,7 @@ pub mod thing { } pub fn go() -> i32 { - let p = crate::thing_h::point { x: point!() }; + let p = crate::thing::point { x: point!() }; p.x } }