Skip to content

Commit b240976

Browse files
RuaRua
committed
refactor: Support associated constants in reorganize_definitions 
1 parent 6c10307 commit b240976

4 files changed

Lines changed: 312 additions & 24 deletions

File tree

‎c2rust-refactor/src/context.rs‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use std::ops::Deref;
44

55
use rustc_ast::ptr::P;
66
use rustc_ast::{
7-
Expr, ExprKind, FnDecl, FnRetTy, ForeignItem, ForeignItemKind, Item, ItemKind, NodeId, Path,
8-
QSelf, UseTreeKind, DUMMY_NODE_ID,
7+
AssocItem, Expr, ExprKind, FnDecl, FnRetTy, ForeignItem, ForeignItemKind, Item, ItemKind,
8+
NodeId, Path, QSelf, UseTreeKind, DUMMY_NODE_ID,
99
};
1010
use rustc_data_structures::fx::FxHashMap;
1111
use rustc_errors::{DiagnosticBuilder, Level};
@@ -1072,6 +1072,16 @@ impl<'a, 'tcx, 'b> TypeCompare<'a, 'tcx, 'b> {
10721072
pub fn compatible_types(&self, item1: &Item, item2: &Item, match_vis: bool) -> bool {
10731073
use rustc_ast::ItemKind::*;
10741074
match (&item1.kind, &item2.kind) {
1075+
(Impl(box ref impl1), Impl(box ref impl2)) => {
1076+
if impl1.items.len() != impl2.items.len() {
1077+
return false;
1078+
}
1079+
1080+
(impl1.items.iter())
1081+
.zip(impl2.items.iter())
1082+
.all(|(item1, item2)| self.compatible_assoc_items(item1, item2, match_vis))
1083+
}
1084+
10751085
// * Assure that these two items are in fact of the same type, just to be safe.
10761086
(TyAlias(box ref ta1), TyAlias(box ref ta2)) => {
10771087
match (
@@ -1226,6 +1236,40 @@ impl<'a, 'tcx, 'b> TypeCompare<'a, 'tcx, 'b> {
12261236
}
12271237
}
12281238

1239+
pub fn compatible_assoc_items(
1240+
&self,
1241+
item1: &AssocItem,
1242+
item2: &AssocItem,
1243+
match_vis: bool,
1244+
) -> bool {
1245+
use rustc_ast::AssocItemKind::*;
1246+
1247+
// Unlike for regular items, associated items must also match by name.
1248+
if item1.ident.as_str() != item2.ident.as_str() {
1249+
return false;
1250+
}
1251+
1252+
match (&item1.kind, &item2.kind) {
1253+
(Const(def1, ty1, expr1), Const(def2, ty2, expr2)) => match (
1254+
self.cx.opt_node_type(item1.id),
1255+
self.cx.opt_node_type(item2.id),
1256+
) {
1257+
(Some(ty1), Some(ty2)) => {
1258+
self.structural_eq_tys(ty1, ty2)
1259+
&& expr1.unnamed_equiv(expr2)
1260+
&& def1.unnamed_equiv(def2)
1261+
}
1262+
_ => {
1263+
self.structural_eq_ast_tys(ty1, ty2, match_vis)
1264+
&& expr1.unnamed_equiv(expr2)
1265+
&& def1.unnamed_equiv(def2)
1266+
}
1267+
},
1268+
1269+
_ => false,
1270+
}
1271+
}
1272+
12291273
/// Compare two function declarations for equivalent argument and return types,
12301274
/// ignoring argument names.
12311275
pub fn compatible_fn_prototypes(&self, decl1: &FnDecl, decl2: &FnDecl) -> bool {

‎c2rust-refactor/src/transform/reorganize_definitions.rs‎

Lines changed: 177 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,79 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> {
287287
keep_items
288288
}
289289

290+
// First, remove and store `impl` items, indexing them by the type they belong to.
291+
let mut impls: HashMap<DefId, MovedDeclImpl> = HashMap::new();
292+
FlatMapNodes::visit(krate, |mut item: P<Item>| {
293+
if let Some((path, _)) = parse_source_header(&item.attrs) {
294+
let header_item = item.clone();
295+
if let ItemKind::Mod(_, ModKind::Loaded(ref mut mod_items, _, _)) = &mut item.kind {
296+
mod_items.retain(|item| {
297+
if let ItemKind::Impl(r#impl) = &item.kind {
298+
// Only keep `impl` items with simple path types, and only if they
299+
// contain nothing but `const` items.
300+
fn impl_parent_decl<'hir>(
301+
cx: &'hir RefactorCtxt,
302+
r#impl: &Impl,
303+
) -> Option<(DefId, &'hir hir::Path<'hir>)>
304+
{
305+
// Only inherent `impl`s that contain no items other than `const`.
306+
if r#impl.of_trait.is_some()
307+
|| !r#impl
308+
.items
309+
.iter()
310+
.all(|item| matches!(item.kind, AssocItemKind::Const(..)))
311+
{
312+
return None;
313+
};
314+
let ty = match cx.hir_map().find(r#impl.self_ty.id)? {
315+
hir::Node::Ty(ty) => ty,
316+
_ => return None,
317+
};
318+
let ty_path = match &ty.kind {
319+
hir::TyKind::Path(hir::QPath::Resolved(None, path)) => path,
320+
_ => return None,
321+
};
322+
match ty_path.res {
323+
Res::Def(_, def_id) => Some((def_id, ty_path)),
324+
_ => None,
325+
}
326+
}
327+
328+
if let Some((decl_def_id, impl_type_path)) =
329+
impl_parent_decl(&self.cx, &r#impl)
330+
{
331+
match impls.entry(decl_def_id) {
332+
Entry::Occupied(_) => warn!(
333+
"decl {decl_def_id:?} ({impl_type_path:?}) \
334+
has more than one `impl` block, dropping the others"
335+
),
336+
Entry::Vacant(entry) => {
337+
let parent_header =
338+
HeaderInfo::new(header_item.ident, path.clone());
339+
entry.insert(MovedDeclImpl {
340+
item: item.clone(),
341+
parent_header,
342+
});
343+
}
344+
}
345+
}
346+
347+
false
348+
} else {
349+
true
350+
}
351+
});
352+
353+
smallvec![item]
354+
} else {
355+
panic!("Unexpected Item kind with header_src attribute");
356+
}
357+
} else {
358+
smallvec![item]
359+
}
360+
});
361+
362+
// Process all the remaining items.
290363
let mut declarations = HeaderDeclarations::new(self.cx);
291364
FlatMapNodes::visit(krate, |mut item: P<Item>| {
292365
if let Some((path, _)) = parse_source_header(&item.attrs) {
@@ -321,8 +394,13 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> {
321394
}
322395
}
323396

397+
let new_def_id = self.cx.node_def_id(item.id);
324398
let header_info = HeaderInfo::new(header_item.ident, path.clone());
325-
let inserted = declarations.insert_item(item.clone(), header_info);
399+
let inserted = declarations.insert_item(
400+
item.clone(),
401+
header_info,
402+
impls.remove(&new_def_id),
403+
);
326404
// Keep the item if we are not collapsing it
327405
!inserted
328406
});
@@ -676,7 +754,7 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> {
676754
} else {
677755
let namespace = self.cx.item_namespace(&item);
678756
if let Some(namespace) = namespace {
679-
match declarations.find_item(item, namespace) {
757+
match declarations.find_item(item, namespace, None) {
680758
ContainsDecl::NotContained => false,
681759
ContainsDecl::Equivalent(_) => true,
682760
ContainsDecl::Definition(_) => true,
@@ -750,6 +828,13 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> {
750828
// Remove src_loc attributes
751829
FlatMapNodes::visit(krate, |mut item: P<Item>| {
752830
item.attrs.retain(|attr| !is_c2rust_attr(attr, "src_loc"));
831+
832+
if let ItemKind::Impl(r#impl) = &mut item.kind {
833+
for item in &mut r#impl.items {
834+
item.attrs.retain(|attr| !is_c2rust_attr(attr, "src_loc"));
835+
}
836+
}
837+
753838
smallvec![item]
754839
});
755840
FlatMapNodes::visit(krate, |mut item: P<ForeignItem>| {
@@ -1085,10 +1170,25 @@ struct MovedDecl {
10851170
namespace: Namespace,
10861171
loc: Option<SrcLoc>,
10871172
parent_header: HeaderInfo,
1173+
1174+
/// The `impl` block that belongs to the definition, if any.
1175+
r#impl: Option<MovedDeclImpl>,
1176+
}
1177+
1178+
#[derive(Debug, Clone)]
1179+
struct MovedDeclImpl {
1180+
item: P<Item>,
1181+
parent_header: HeaderInfo,
10881182
}
10891183

10901184
impl MovedDecl {
1091-
fn new<T>(decl: T, def_id: DefId, namespace: Namespace, parent_header: HeaderInfo) -> Self
1185+
fn new<T>(
1186+
decl: T,
1187+
def_id: DefId,
1188+
namespace: Namespace,
1189+
parent_header: HeaderInfo,
1190+
r#impl: Option<MovedDeclImpl>,
1191+
) -> Self
10921192
where
10931193
T: Into<DeclKind>,
10941194
{
@@ -1105,6 +1205,7 @@ impl MovedDecl {
11051205
namespace,
11061206
loc,
11071207
parent_header,
1208+
r#impl,
11081209
}
11091210
}
11101211

@@ -1291,14 +1392,12 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
12911392

12921393
/// Add an item into the module. If it has a name conflict with an existing
12931394
/// item, choose the definition item over any declarations.
1294-
pub fn insert_item(&mut self, mut item: P<Item>, parent_header: HeaderInfo) -> bool {
1295-
let namespace = self.cx.item_namespace(&item);
1296-
let new_def_id = self.cx.node_def_id(item.id);
1297-
let ident = if let ItemKind::Use(tree) = &item.kind {
1298-
tree.ident()
1299-
} else {
1300-
item.ident
1301-
};
1395+
pub fn insert_item(
1396+
&mut self,
1397+
mut item: P<Item>,
1398+
parent_header: HeaderInfo,
1399+
r#impl: Option<MovedDeclImpl>,
1400+
) -> bool {
13021401
match &item.kind {
13031402
// We have to disambiguate anonymous items by contents,
13041403
// since we don't have a proper Ident.
@@ -1307,7 +1406,7 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
13071406
// ident_map.
13081407
ItemKind::Use(tree) if is_nested(tree) => {
13091408
for u in split_uses(item).into_iter() {
1310-
self.insert_item(u, parent_header.clone());
1409+
self.insert_item(u, parent_header.clone(), r#impl.clone());
13111410
}
13121411
true
13131412
}
@@ -1335,11 +1434,24 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
13351434
// we don't have any items with the same name but different
13361435
// contents.
13371436
_ => {
1437+
let namespace = self.cx.item_namespace(&item);
1438+
let new_def_id = self.cx.node_def_id(item.id);
1439+
let ident = if let ItemKind::Use(tree) = &item.kind {
1440+
tree.ident()
1441+
} else {
1442+
item.ident
1443+
};
13381444
let unnamed = ident.as_str().contains("C2Rust_Unnamed");
1339-
let def_id_mapping = match self.find_item(&item, namespace.unwrap()) {
1445+
let impl_item = r#impl.as_ref().map(|r#impl| &*r#impl.item);
1446+
let def_id_mapping = match self.find_item(&item, namespace.unwrap(), impl_item) {
13401447
ContainsDecl::NotContained => {
1341-
let new_item =
1342-
MovedDecl::new(item, new_def_id, namespace.unwrap(), parent_header);
1448+
let new_item = MovedDecl::new(
1449+
item,
1450+
new_def_id,
1451+
namespace.unwrap(),
1452+
parent_header,
1453+
r#impl,
1454+
);
13431455
if unnamed {
13441456
self.unnamed_items[namespace.unwrap()].push(new_item);
13451457
} else {
@@ -1359,17 +1471,27 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
13591471
ContainsDecl::Use(existing) => {
13601472
let existing_def_id = existing.def_id;
13611473
existing.join_visibility(&item.vis.kind);
1362-
*existing =
1363-
MovedDecl::new(item, new_def_id, namespace.unwrap(), parent_header);
1474+
*existing = MovedDecl::new(
1475+
item,
1476+
new_def_id,
1477+
namespace.unwrap(),
1478+
parent_header,
1479+
r#impl,
1480+
);
13641481
Some((existing_def_id, new_def_id))
13651482
}
13661483

13671484
ContainsDecl::Equivalent(existing) if existing.is_foreign() => {
13681485
let existing_def_id = existing.def_id;
13691486
item.vis.kind =
13701487
join_visibility(&existing.visibility().kind, &item.vis.kind);
1371-
*existing =
1372-
MovedDecl::new(item, new_def_id, namespace.unwrap(), parent_header);
1488+
*existing = MovedDecl::new(
1489+
item,
1490+
new_def_id,
1491+
namespace.unwrap(),
1492+
parent_header,
1493+
r#impl,
1494+
);
13731495
Some((existing_def_id, new_def_id))
13741496
}
13751497

@@ -1395,6 +1517,7 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
13951517
new_def_id,
13961518
namespace,
13971519
parent_header.clone(),
1520+
None,
13981521
);
13991522
if unnamed {
14001523
self.unnamed_items[namespace].push(new_item);
@@ -1414,6 +1537,7 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
14141537
new_def_id,
14151538
namespace,
14161539
parent_header.clone(),
1540+
None,
14171541
);
14181542
Some((existing_def_id, new_def_id))
14191543
}
@@ -1507,6 +1631,17 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
15071631
foreign_items.entry(abi).or_default().push(fi);
15081632
}
15091633
}
1634+
1635+
// If there is an impl item, add it now.
1636+
if let Some(r#impl) = item.r#impl {
1637+
let cur_mod_name = r#impl.parent_header.ident;
1638+
let i = r#impl.item;
1639+
if last_item_mod != Some(cur_mod_name) {
1640+
st.add_comment(i.id, make_header_comment(last_item_mod, cur_mod_name));
1641+
last_item_mod = Some(cur_mod_name);
1642+
}
1643+
items.push(i);
1644+
}
15101645
}
15111646

15121647
let foreign_mods = foreign_items
@@ -1516,14 +1651,30 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
15161651
foreign_mods.chain(items.into_iter()).collect()
15171652
}
15181653

1519-
fn find_item<'b>(&'b mut self, item: &Item, namespace: Namespace) -> ContainsDecl<'b> {
1654+
fn find_item<'b>(
1655+
&'b mut self,
1656+
item: &Item,
1657+
namespace: Namespace,
1658+
impl_item: Option<&Item>,
1659+
) -> ContainsDecl<'b> {
15201660
let ident = if let ItemKind::Use(tree) = &item.kind {
15211661
tree.ident()
15221662
} else {
15231663
item.ident
15241664
};
15251665
assert!(ident.name != kw::Empty);
15261666

1667+
let impl_is_compatible = |existing_decl: &MovedDecl| -> bool {
1668+
match (impl_item, &existing_decl.r#impl) {
1669+
(None, None) => true,
1670+
(Some(impl_item), Some(existing_impl)) => {
1671+
self.cx
1672+
.compatible_types(impl_item, &existing_impl.item, false)
1673+
}
1674+
_ => false,
1675+
}
1676+
};
1677+
15271678
if ident.as_str().contains("C2Rust_Unnamed") {
15281679
for existing_decl in self.unnamed_items[namespace].iter_mut() {
15291680
match &existing_decl.kind {
@@ -1534,7 +1685,9 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
15341685
| ItemKind::Enum(..) => {
15351686
// Does the new item match the existing item, except
15361687
// for unnamed names?
1537-
if item.kind.unnamed_equiv(&existing_item.kind) {
1688+
if item.kind.unnamed_equiv(&existing_item.kind)
1689+
&& impl_is_compatible(existing_decl)
1690+
{
15381691
return ContainsDecl::Equivalent(existing_decl);
15391692
}
15401693
}
@@ -1579,7 +1732,9 @@ impl<'a, 'tcx> HeaderDeclarations<'a, 'tcx> {
15791732
// Otherwise make sure these items are structurally
15801733
// equivalent.
15811734
_ => {
1582-
if self.cx.compatible_types(&item, &existing_item, true) {
1735+
if self.cx.compatible_types(&item, &existing_item, true)
1736+
&& impl_is_compatible(existing_decl)
1737+
{
15831738
return ContainsDecl::Equivalent(existing_decl);
15841739
}
15851740
}

0 commit comments

Comments
 (0)