diff --git a/c2rust-refactor/src/context.rs b/c2rust-refactor/src/context.rs index 557d254c99..3ef489993d 100644 --- a/c2rust-refactor/src/context.rs +++ b/c2rust-refactor/src/context.rs @@ -4,8 +4,8 @@ use std::ops::Deref; use rustc_ast::ptr::P; use rustc_ast::{ - Expr, ExprKind, FnDecl, FnRetTy, ForeignItem, ForeignItemKind, Item, ItemKind, NodeId, Path, - QSelf, UseTreeKind, DUMMY_NODE_ID, + AssocItem, Expr, ExprKind, FnDecl, FnRetTy, ForeignItem, ForeignItemKind, Item, ItemKind, + NodeId, Path, QSelf, UseTreeKind, DUMMY_NODE_ID, }; use rustc_data_structures::fx::FxHashMap; use rustc_errors::{DiagnosticBuilder, Level}; @@ -1073,6 +1073,16 @@ impl<'a, 'tcx, 'b> TypeCompare<'a, 'tcx, 'b> { pub fn compatible_types(&self, item1: &Item, item2: &Item, match_vis: bool) -> bool { use rustc_ast::ItemKind::*; match (&item1.kind, &item2.kind) { + (Impl(box ref impl1), Impl(box ref impl2)) => { + if impl1.items.len() != impl2.items.len() { + return false; + } + + (impl1.items.iter()) + .zip(impl2.items.iter()) + .all(|(item1, item2)| self.compatible_assoc_items(item1, item2, match_vis)) + } + // * Assure that these two items are in fact of the same type, just to be safe. (TyAlias(box ref ta1), TyAlias(box ref ta2)) => { match ( @@ -1227,6 +1237,40 @@ impl<'a, 'tcx, 'b> TypeCompare<'a, 'tcx, 'b> { } } + pub fn compatible_assoc_items( + &self, + item1: &AssocItem, + item2: &AssocItem, + match_vis: bool, + ) -> bool { + use rustc_ast::AssocItemKind::*; + + // Unlike for regular items, associated items must also match by name. + if item1.ident.as_str() != item2.ident.as_str() { + return false; + } + + match (&item1.kind, &item2.kind) { + (Const(def1, ty1, expr1), Const(def2, ty2, expr2)) => match ( + self.cx.opt_node_type(item1.id), + self.cx.opt_node_type(item2.id), + ) { + (Some(ty1), Some(ty2)) => { + self.structural_eq_tys(ty1, ty2) + && expr1.unnamed_equiv(expr2) + && def1.unnamed_equiv(def2) + } + _ => { + self.structural_eq_ast_tys(ty1, ty2, match_vis) + && expr1.unnamed_equiv(expr2) + && def1.unnamed_equiv(def2) + } + }, + + _ => false, + } + } + /// Compare two function declarations for equivalent argument and return types, /// ignoring argument names. pub fn compatible_fn_prototypes(&self, decl1: &FnDecl, decl2: &FnDecl) -> bool { diff --git a/c2rust-refactor/src/transform/reorganize_definitions.rs b/c2rust-refactor/src/transform/reorganize_definitions.rs index f9a2bf6143..8a551d45da 100644 --- a/c2rust-refactor/src/transform/reorganize_definitions.rs +++ b/c2rust-refactor/src/transform/reorganize_definitions.rs @@ -289,10 +289,87 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> { keep_items } + // First, remove and store `impl` items, indexing them by the type they belong to. + let mut impls: HashMap = HashMap::new(); + FlatMapNodes::visit(krate, |mut item: P| { + if let Some((path, _)) = parse_source_header(&item.attrs) { + let header_ident = item.ident; + if let ItemKind::Mod(_, ModKind::Loaded(ref mut mod_items, _, _)) = &mut item.kind { + mod_items.retain(|item| { + if let ItemKind::Impl(r#impl) = &item.kind { + // Only keep `impl` items with simple path types, and only if they + // contain nothing but `const` items. + fn impl_parent_decl<'hir>( + cx: &'hir RefactorCtxt, + r#impl: &Impl, + ) -> Option<(DefId, &'hir hir::Path<'hir>)> + { + // Only inherent `impl`s that contain no items other than `const`. + if r#impl.of_trait.is_some() + || !r#impl + .items + .iter() + .all(|item| matches!(item.kind, AssocItemKind::Const(..))) + { + return None; + }; + let ty = match cx.hir_map().find(r#impl.self_ty.id)? { + hir::Node::Ty(ty) => ty, + _ => return None, + }; + let ty_path = match &ty.kind { + hir::TyKind::Path(hir::QPath::Resolved(None, path)) => path, + _ => return None, + }; + match ty_path.res { + Res::Def(_, def_id) => Some((def_id, ty_path)), + _ => None, + } + } + + if let Some((decl_def_id, impl_type_path)) = + impl_parent_decl(&self.cx, &r#impl) + { + match impls.entry(decl_def_id) { + Entry::Occupied(_) => warn!( + "decl {decl_def_id:?} ({impl_type_path:?}) \ + has more than one `impl` block, dropping the others" + ), + Entry::Vacant(entry) => { + let parent_header = + HeaderInfo::new(header_ident, path.clone()); + entry.insert(MovedDeclImpl { + item: item.clone(), + parent_header, + }); + } + } + } + + false + } else { + true + } + }); + + if mod_items.is_empty() { + smallvec![] + } else { + smallvec![item] + } + } else { + panic!("Unexpected Item kind with header_src attribute"); + } + } else { + smallvec![item] + } + }); + + // Process all the remaining items. let mut declarations = HeaderDeclarations::new(self.cx); FlatMapNodes::visit(krate, |mut item: P| { if let Some((path, _)) = parse_source_header(&item.attrs) { - let header_item = item.clone(); + let header_ident = item.ident; // TODO: handle use's at the top of the crate if let ItemKind::Mod(_, ModKind::Loaded(ref mut mod_items, _, _)) = &mut item.kind { // Split complex uses before iterating over the items @@ -323,8 +400,13 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> { } } - let header_info = HeaderInfo::new(header_item.ident, path.clone()); - let inserted = declarations.insert_item(item.clone(), header_info); + let new_def_id = self.cx.node_def_id(item.id); + let header_info = HeaderInfo::new(header_ident, path.clone()); + let inserted = declarations.insert_item( + item.clone(), + header_info, + impls.remove(&new_def_id), + ); // Keep the item if we are not collapsing it !inserted }); @@ -678,7 +760,7 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> { } else { let namespace = self.cx.item_namespace(&item); if let Some(namespace) = namespace { - match declarations.find_item(item, namespace) { + match declarations.find_item(item, namespace, None) { ContainsDecl::NotContained => false, ContainsDecl::Equivalent(_) => true, ContainsDecl::Definition(_) => true, @@ -752,6 +834,13 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> { // Remove src_loc attributes FlatMapNodes::visit(krate, |mut item: P| { item.attrs.retain(|attr| !is_c2rust_attr(attr, "src_loc")); + + if let ItemKind::Impl(r#impl) = &mut item.kind { + for item in &mut r#impl.items { + item.attrs.retain(|attr| !is_c2rust_attr(attr, "src_loc")); + } + } + smallvec![item] }); FlatMapNodes::visit(krate, |mut item: P| { @@ -1123,10 +1212,25 @@ struct MovedDecl { namespace: Namespace, loc: Option, parent_header: HeaderInfo, + + /// The `impl` block that belongs to the definition, if any. + r#impl: Option, +} + +#[derive(Debug, Clone)] +struct MovedDeclImpl { + item: P, + parent_header: HeaderInfo, } impl MovedDecl { - fn new(decl: T, def_id: DefId, namespace: Namespace, parent_header: HeaderInfo) -> Self + fn new( + decl: T, + def_id: DefId, + namespace: Namespace, + parent_header: HeaderInfo, + r#impl: Option, + ) -> Self where T: Into, { @@ -1143,6 +1247,7 @@ impl MovedDecl { namespace, loc, parent_header, + r#impl, } } @@ -1329,14 +1434,12 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { /// Add an item into the module. If it has a name conflict with an existing /// item, choose the definition item over any declarations. - pub fn insert_item(&mut self, mut item: P, parent_header: HeaderInfo) -> bool { - let namespace = self.cx.item_namespace(&item); - let new_def_id = self.cx.node_def_id(item.id); - let ident = if let ItemKind::Use(tree) = &item.kind { - tree.ident() - } else { - item.ident - }; + pub fn insert_item( + &mut self, + mut item: P, + parent_header: HeaderInfo, + r#impl: Option, + ) -> bool { match &item.kind { // We have to disambiguate anonymous items by contents, // since we don't have a proper Ident. @@ -1345,7 +1448,7 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { // ident_map. ItemKind::Use(tree) if is_nested(tree) => { for u in split_uses(item).into_iter() { - self.insert_item(u, parent_header.clone()); + self.insert_item(u, parent_header.clone(), r#impl.clone()); } true } @@ -1373,11 +1476,24 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { // we don't have any items with the same name but different // contents. _ => { + let namespace = self.cx.item_namespace(&item); + let new_def_id = self.cx.node_def_id(item.id); + let ident = if let ItemKind::Use(tree) = &item.kind { + tree.ident() + } else { + item.ident + }; let unnamed = ident.as_str().contains("C2Rust_Unnamed"); - let def_id_mapping = match self.find_item(&item, namespace.unwrap()) { + let impl_item = r#impl.as_ref().map(|r#impl| &*r#impl.item); + let def_id_mapping = match self.find_item(&item, namespace.unwrap(), impl_item) { ContainsDecl::NotContained => { - let new_item = - MovedDecl::new(item, new_def_id, namespace.unwrap(), parent_header); + let new_item = MovedDecl::new( + item, + new_def_id, + namespace.unwrap(), + parent_header, + r#impl, + ); if unnamed { self.unnamed_items[namespace.unwrap()].push(new_item); } else { @@ -1397,8 +1513,13 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { ContainsDecl::Use(existing) => { let existing_def_id = existing.def_id; existing.join_visibility(&item.vis.kind); - *existing = - MovedDecl::new(item, new_def_id, namespace.unwrap(), parent_header); + *existing = MovedDecl::new( + item, + new_def_id, + namespace.unwrap(), + parent_header, + r#impl, + ); Some((existing_def_id, new_def_id)) } @@ -1406,8 +1527,13 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { let existing_def_id = existing.def_id; item.vis.kind = join_visibility(&existing.visibility().kind, &item.vis.kind); - *existing = - MovedDecl::new(item, new_def_id, namespace.unwrap(), parent_header); + *existing = MovedDecl::new( + item, + new_def_id, + namespace.unwrap(), + parent_header, + r#impl, + ); Some((existing_def_id, new_def_id)) } @@ -1433,6 +1559,7 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { new_def_id, namespace, parent_header.clone(), + None, ); if unnamed { self.unnamed_items[namespace].push(new_item); @@ -1452,6 +1579,7 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { new_def_id, namespace, parent_header.clone(), + None, ); Some((existing_def_id, new_def_id)) } @@ -1545,6 +1673,17 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { foreign_items.entry(abi).or_default().push(fi); } } + + // If there is an impl item, add it now. + if let Some(r#impl) = item.r#impl { + let cur_mod_name = r#impl.parent_header.ident; + let i = r#impl.item; + if last_item_mod != Some(cur_mod_name) { + st.add_comment(i.id, make_header_comment(last_item_mod, cur_mod_name)); + last_item_mod = Some(cur_mod_name); + } + items.push(i); + } } let foreign_mods = foreign_items @@ -1554,7 +1693,12 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { foreign_mods.chain(items.into_iter()).collect() } - fn find_item<'b>(&'b mut self, item: &Item, namespace: Namespace) -> ContainsDecl<'b> { + fn find_item<'b>( + &'b mut self, + item: &Item, + namespace: Namespace, + impl_item: Option<&Item>, + ) -> ContainsDecl<'b> { let ident = if let ItemKind::Use(tree) = &item.kind { tree.ident() } else { @@ -1562,6 +1706,17 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { }; assert!(ident.name != kw::Empty); + let impl_is_compatible = |existing_decl: &MovedDecl| -> bool { + match (impl_item, &existing_decl.r#impl) { + (None, None) => true, + (Some(impl_item), Some(existing_impl)) => { + self.cx + .compatible_types(impl_item, &existing_impl.item, false) + } + _ => false, + } + }; + if ident.as_str().contains("C2Rust_Unnamed") { for existing_decl in self.unnamed_items[namespace].iter_mut() { match &existing_decl.kind { @@ -1572,7 +1727,9 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { | ItemKind::Enum(..) => { // Does the new item match the existing item, except // for unnamed names? - if item.kind.unnamed_equiv(&existing_item.kind) { + if item.kind.unnamed_equiv(&existing_item.kind) + && impl_is_compatible(existing_decl) + { return ContainsDecl::Equivalent(existing_decl); } } @@ -1617,7 +1774,9 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> { // Otherwise make sure these items are structurally // equivalent. _ => { - if self.cx.compatible_types(&item, &existing_item, true) { + if self.cx.compatible_types(&item, &existing_item, true) + && impl_is_compatible(existing_decl) + { return ContainsDecl::Equivalent(existing_decl); } } diff --git a/c2rust-refactor/tests/snapshots.rs b/c2rust-refactor/tests/snapshots.rs index 68c93b5acb..89fe0de2c5 100644 --- a/c2rust-refactor/tests/snapshots.rs +++ b/c2rust-refactor/tests/snapshots.rs @@ -445,6 +445,13 @@ fn test_reorganize_definitions() { .test(); } +#[test] +fn test_reorganize_assoc_items() { + refactor("reorganize_definitions") + .named("reorganize_assoc_items.rs") + .test(); +} + #[test] fn test_reorganize_foreign_types() { refactor("reorganize_definitions") diff --git a/c2rust-refactor/tests/snapshots/reorganize_assoc_items.rs b/c2rust-refactor/tests/snapshots/reorganize_assoc_items.rs new file mode 100644 index 0000000000..2fee527925 --- /dev/null +++ b/c2rust-refactor/tests/snapshots/reorganize_assoc_items.rs @@ -0,0 +1,79 @@ +#![feature(extern_types)] +#![feature(rustc_private)] +#![feature(register_tool)] +#![register_tool(c2rust)] +#![allow(non_upper_case_globals)] +#![allow(non_camel_case_types)] +#![allow(non_snake_case)] +#![allow(dead_code)] +#![allow(mutable_transmutes)] +#![allow(unused_mut)] + +pub mod bar { + #[c2rust::header_src = "/home/user/some/workspace/foobar/bar.h:5"] + pub mod bar_h { + #[derive(Clone, Copy)] + #[repr(transparent)] + #[c2rust::src_loc = "14:0"] + pub struct C2Rust_Unnamed_2(pub ::core::ffi::c_uint); + + #[c2rust::src_loc = "14:0"] + impl C2Rust_Unnamed_2 { + #[c2rust::src_loc = "15:0"] + pub const U: Self = Self(42); + } + + #[derive(Clone, Copy)] + #[repr(transparent)] + #[c2rust::src_loc = "11:0"] + pub struct SomeEnum(pub ::core::ffi::c_uint); + + #[c2rust::src_loc = "11:0"] + impl SomeEnum { + #[c2rust::src_loc = "12:0"] + pub const A: Self = Self(0); + #[c2rust::src_loc = "13:0"] + pub const B: Self = Self(1); + } + } +} + +pub mod foo { + #[c2rust::header_src = "/home/user/some/workspace/foobar/bar.h:5"] + pub mod bar_h { + #[derive(Clone, Copy)] + #[repr(transparent)] + #[c2rust::src_loc = "4:0"] + pub struct C2Rust_Unnamed_3(pub ::core::ffi::c_uint); + + #[c2rust::src_loc = "4:0"] + impl C2Rust_Unnamed_3 { + #[c2rust::src_loc = "5:0"] + pub const V: Self = Self(42); + } + + #[derive(Clone, Copy)] + #[repr(transparent)] + #[c2rust::src_loc = "1:0"] + pub struct SomeEnum(pub ::core::ffi::c_uint); + + #[c2rust::src_loc = "1:0"] + impl SomeEnum { + #[c2rust::src_loc = "2:0"] + pub const A: Self = Self(0); + #[c2rust::src_loc = "3:0"] + pub const B: Self = Self(1); + } + } + + unsafe fn foo() { + let e1 = super::foo::bar_h::SomeEnum::A; + let e2 = super::bar::bar_h::SomeEnum::B; + let e3 = super::bar::bar_h::C2Rust_Unnamed_2::U; + let e4 = super::foo::bar_h::C2Rust_Unnamed_3::V; + } +} + +fn main() { + println!("hello!"); +} diff --git a/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_assoc_items.rs.snap b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_assoc_items.rs.snap new file mode 100644 index 0000000000..f905a81a40 --- /dev/null +++ b/c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_assoc_items.rs.snap @@ -0,0 +1,59 @@ +--- +source: c2rust-refactor/tests/snapshots.rs +expression: c2rust-refactor reorganize_definitions --rewrite-mode alongside -- tests/snapshots/reorganize_assoc_items.rs --edition 2021 +--- +#![feature(extern_types)] +#![feature(rustc_private)] +#![feature(register_tool)] +#![register_tool(c2rust)] +#![allow(non_upper_case_globals)] +#![allow(non_camel_case_types)] +#![allow(non_snake_case)] +#![allow(dead_code)] +#![allow(mutable_transmutes)] +#![allow(unused_mut)] + +pub mod bar_h { + #[derive(Clone, Copy)] + #[repr(transparent)] + pub struct C2Rust_Unnamed_3(pub ::core::ffi::c_uint); + + impl crate::bar_h::C2Rust_Unnamed_3 { + pub const V: Self = Self(42); + } + + #[derive(Clone, Copy)] + #[repr(transparent)] + + pub struct SomeEnum(pub ::core::ffi::c_uint); + + impl crate::bar_h::SomeEnum { + pub const A: Self = Self(0); + + pub const B: Self = Self(1); + } + + #[derive(Clone, Copy)] + #[repr(transparent)] + + pub struct C2Rust_Unnamed_2(pub ::core::ffi::c_uint); + + impl crate::bar_h::C2Rust_Unnamed_2 { + pub const U: Self = Self(42); + } +} +pub mod bar {} + +pub mod foo { + + unsafe fn foo() { + let e1 = crate::bar_h::SomeEnum::A; + let e2 = crate::bar_h::SomeEnum::B; + let e3 = crate::bar_h::C2Rust_Unnamed_2::U; + let e4 = crate::bar_h::C2Rust_Unnamed_3::V; + } +} + +fn main() { + println!("hello!"); +}