Skip to content

Commit

Permalink
Auto merge of rust-lang#3835 - JoJoDeveloping:tb-fix-stack-overflow, …
Browse files Browse the repository at this point in the history
…r=RalfJung

Avoid extra copy by using `retain_mut` and moving the deletion into the closure

Fixes the FIXME introduced in rust-lang#3833. Thanks to `@dmitrii-ubskii` for the idea 🙂
  • Loading branch information
bors committed Aug 22, 2024
2 parents e881c42 + 3a423fc commit b8c02eb
Showing 1 changed file with 24 additions and 21 deletions.
45 changes: 24 additions & 21 deletions src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -754,29 +754,32 @@ impl Tree {
// list of remaining children back in.
let mut children_of_node =
mem::take(&mut self.nodes.get_mut(*tag).unwrap().children);
// Remove all useless children, and save them for later.
// The closure needs `&self` and the loop below needs `&mut self`, so we need to `collect`
// in to a temporary list.
let to_remove: Vec<_> =
children_of_node.drain_filter(|x| self.is_useless(*x, live)).collect();
// Remove all useless children.
children_of_node.retain_mut(|idx| {
if self.is_useless(*idx, live) {
// Note: In the rest of this comment, "this node" refers to `idx`.
// This node has no more children (if there were any, they have already been removed).
// It is also unreachable as determined by the GC, so we can remove it everywhere.
// Due to the API of UniMap we must make sure to call
// `UniValMap::remove` for the key of this node on *all* maps that used it
// (which are `self.nodes` and every range of `self.rperms`)
// before we can safely apply `UniKeyMap::remove` to truly remove
// this tag from the `tag_mapping`.
let node = self.nodes.remove(*idx).unwrap();
for (_perms_range, perms) in self.rperms.iter_mut_all() {
perms.remove(*idx);
}
self.tag_mapping.remove(&node.tag);
// now delete it
false
} else {
// do nothing, but retain
true
}
});
// Put back the now-filtered vector.
self.nodes.get_mut(*tag).unwrap().children = children_of_node;
// Now, all that is left is unregistering the children saved in `to_remove`.
for idx in to_remove {
// Note: In the rest of this comment, "this node" refers to `idx`.
// This node has no more children (if there were any, they have already been removed).
// It is also unreachable as determined by the GC, so we can remove it everywhere.
// Due to the API of UniMap we must make sure to call
// `UniValMap::remove` for the key of this node on *all* maps that used it
// (which are `self.nodes` and every range of `self.rperms`)
// before we can safely apply `UniKeyMap::remove` to truly remove
// this tag from the `tag_mapping`.
let node = self.nodes.remove(idx).unwrap();
for (_perms_range, perms) in self.rperms.iter_mut_all() {
perms.remove(idx);
}
self.tag_mapping.remove(&node.tag);
}

// We are done, the parent can continue.
stack.pop();
continue;
Expand Down

0 comments on commit b8c02eb

Please sign in to comment.