Skip to content

Commit c8a7b42

Browse files
perfloop-agentperfloop-agent
committed
Run map element destructors during refcount iterator erase
Signed-off-by: Perfloop Agent <agent@perfloop.ai> Signed-off-by: Tomás Senart <ts@perfloop.ai>
1 parent a65f73b commit c8a7b42

6 files changed

Lines changed: 167 additions & 30 deletions

File tree

‎docs/src/rules/writing-rules.md‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,19 @@ std::string f9(std::size_t n, char ch) { return std::string(n, ch); }
102102
Overloads that differ in value category are distinct rules too: `rules/vector`
103103
has separate rules for `push_back(const T1 &)` and `push_back(T1 &&)`.
104104
105-
No destructor rules exist so far: the STL and libc APIs covered by the current
106-
rules have not needed any, since their types map to Rust types whose `Drop`
107-
implementations already do the right thing.
105+
Destruction of a translated record uses the explicit `Destructor` trait, not
106+
Rust's `Drop`. Container rules must therefore distinguish releasing Rust storage
107+
from calling a contained record's C++ destructor. For example, the refcount map
108+
iterator-erase rules (`f3` and `f27` in
109+
[`rules/map/tgt_refcount.rs`](https://github.com/Cpp2Rust/cpp2rust/blob/master/rules/map/tgt_refcount.rs))
110+
pass a finalizer to `Map::erase`. The runtime unlinks the entry and removes its
111+
key from the index, then calls the finalizer with pointers to the stored key and
112+
mapped value, without holding a borrow of the map. The rules reuse the
113+
`__swap::Destroy` dispatch to call each type's destructor if it has one, and do
114+
nothing for trivial types. This dispatch is in the expanded rule, where the
115+
concrete types are known; a `Destructor` bound on the generic runtime method
116+
would exclude trivial types. Iterator copies may retain the Rust storage, but do
117+
not delay or repeat these explicit destructor calls.
108118
109119
## Operators
110120

‎rules/map/shim.rs‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -255,18 +255,23 @@ impl<K: CppLess + 'static, V: 'static> Map<K, Entry<K, V>> {
255255
pub fn erase(
256256
map: Ptr<Self>,
257257
it: &BidirectionalIterator<Entry<K, V>>,
258+
destroy: impl FnOnce(Ptr<K>, Ptr<V>),
258259
) -> BidirectionalIterator<Entry<K, V>> {
259260
let BidirectionalIterator::Map(node) = it;
260-
let key = node
261-
.borrow()
262-
.value
263-
.as_ref()
264-
.expect("ub: erase of end iterator")
265-
.borrow()
266-
.0
267-
.as_pointer();
261+
let (key, value) = {
262+
let node = node.borrow();
263+
let entry = node
264+
.value
265+
.as_ref()
266+
.expect("ub: erase of end iterator")
267+
.borrow();
268+
(entry.0.as_pointer(), entry.1.as_pointer())
269+
};
268270
let next = Self::unlink(node);
269-
map.with_mut(|map| map.nodes.remove(&MapKey(key)));
271+
map.with_mut(|map| map.nodes.remove(&MapKey(key.clone())));
272+
// Run C++ destructors after removing the key from the index and ending
273+
// the map borrow. Iterator copies may still keep the node alive.
274+
destroy(key, value);
270275
BidirectionalIterator::Map(next)
271276
}
272277

‎rules/map/tgt_refcount.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ fn f3<T1: CppLess + 'static, T2: 'static>(
2424
a0: Ptr<Map<T1, (Value<T1>, Value<T2>)>>,
2525
a1: BidirectionalIterator<(Value<T1>, Value<T2>)>,
2626
) -> BidirectionalIterator<(Value<T1>, Value<T2>)> {
27-
Map::erase(a0, &a1)
27+
Map::erase(a0, &a1, |__key, __value| {
28+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
29+
(&&Destroy(&__value)).destroy();
30+
(&&Destroy(&__key)).destroy();
31+
})
2832
}
2933

3034
fn f5<T1, T2>() -> Map<T1, (Value<T1>, Value<T2>)> {
@@ -87,7 +91,11 @@ fn f27<T1: CppLess + 'static, T2: 'static>(
8791
a0: Ptr<Map<T1, (Value<T1>, Value<T2>)>>,
8892
a1: BidirectionalIterator<(Value<T1>, Value<T2>)>,
8993
) -> BidirectionalIterator<(Value<T1>, Value<T2>)> {
90-
Map::erase(a0, &a1)
94+
Map::erase(a0, &a1, |__key, __value| {
95+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
96+
(&&Destroy(&__value)).destroy();
97+
(&&Destroy(&__key)).destroy();
98+
})
9199
}
92100

93101
fn f28<T1: CppLess + 'static, T2: 'static>(a0: Ptr<Map<T1, (Value<T1>, Value<T2>)>>, a1: Ptr<T1>) -> usize {

‎tests/unit/out/refcount/map.rs‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,15 @@ fn main_0() -> i32 {
315315
.as_pointer()
316316
.with(|__m: &Map<i16, (Value<i16>, Value<u32>)>| __m.end())
317317
);
318-
Map::erase((r).clone(), &({ (*it4.borrow()).clone() }));
318+
Map::erase(
319+
(r).clone(),
320+
&({ (*it4.borrow()).clone() }),
321+
|__key, __value| {
322+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
323+
(&&Destroy(&__value)).destroy();
324+
(&&Destroy(&__key)).destroy();
325+
},
326+
);
319327
assert!(({ ((*r.upgrade().deref()).len() == 3_usize) }));
320328
assert!(
321329
({
@@ -370,9 +378,16 @@ fn main_0() -> i32 {
370378
}),
371379
),
372380
));
373-
let next: Value<BidirectionalIterator<(Value<i16>, Value<u32>)>> = Rc::new(RefCell::new(
374-
Map::erase(m.as_pointer(), &({ (*cit.borrow()).clone() })),
375-
));
381+
let next: Value<BidirectionalIterator<(Value<i16>, Value<u32>)>> =
382+
Rc::new(RefCell::new(Map::erase(
383+
m.as_pointer(),
384+
&({ (*cit.borrow()).clone() }),
385+
|__key, __value| {
386+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
387+
(&&Destroy(&__value)).destroy();
388+
(&&Destroy(&__key)).destroy();
389+
},
390+
)));
376391
assert!(
377392
(*next.borrow())
378393
== ({
@@ -383,7 +398,15 @@ fn main_0() -> i32 {
383398
})
384399
})
385400
);
386-
(*next.borrow_mut()) = Map::erase(m.as_pointer(), &({ (*next.borrow()).clone() }));
401+
(*next.borrow_mut()) = Map::erase(
402+
m.as_pointer(),
403+
&({ (*next.borrow()).clone() }),
404+
|__key, __value| {
405+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
406+
(&&Destroy(&__value)).destroy();
407+
(&&Destroy(&__key)).destroy();
408+
},
409+
);
387410
assert!(
388411
(*next.borrow())
389412
== m.as_pointer()

‎tests/unit/out/refcount/map_erase_destructor.rs‎

Lines changed: 97 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ pub fn check_erase_2(mut key_destructor: bool, mut value_destructor: bool) {
101101
Rc::new(RefCell::new(Map::erase(
102102
m.as_pointer(),
103103
&({ (*middle.borrow()).clone() }),
104+
|__key, __value| {
105+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
106+
(&&Destroy(&__value)).destroy();
107+
(&&Destroy(&__key)).destroy();
108+
},
104109
)));
105110
assert!(
106111
(({
@@ -175,7 +180,15 @@ pub fn check_erase_2(mut key_destructor: bool, mut value_destructor: bool) {
175180
),
176181
),
177182
);
178-
(*next.borrow_mut()) = Map::erase(m.as_pointer(), &({ (*const_first.borrow()).clone() }));
183+
(*next.borrow_mut()) = Map::erase(
184+
m.as_pointer(),
185+
&({ (*const_first.borrow()).clone() }),
186+
|__key, __value| {
187+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
188+
(&&Destroy(&__value)).destroy();
189+
(&&Destroy(&__key)).destroy();
190+
},
191+
);
179192
assert!(
180193
(({
181194
let __idx0 = (1) as usize;
@@ -204,7 +217,15 @@ pub fn check_erase_2(mut key_destructor: bool, mut value_destructor: bool) {
204217
} == { (last_value).clone() })
205218
})
206219
);
207-
(*next.borrow_mut()) = Map::erase(m.as_pointer(), &({ (*last.borrow()).clone() }));
220+
(*next.borrow_mut()) = Map::erase(
221+
m.as_pointer(),
222+
&({ (*last.borrow()).clone() }),
223+
|__key, __value| {
224+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
225+
(&&Destroy(&__value)).destroy();
226+
(&&Destroy(&__key)).destroy();
227+
},
228+
);
208229
assert!(
209230
(({
210231
let __idx0 = (3) as usize;
@@ -295,6 +316,11 @@ pub fn check_erase_3(mut key_destructor: bool, mut value_destructor: bool) {
295316
Rc::new(RefCell::new(Map::erase(
296317
m.as_pointer(),
297318
&({ (*middle.borrow()).clone() }),
319+
|__key, __value| {
320+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
321+
(&&Destroy(&__value)).destroy();
322+
(&&Destroy(&__key)).destroy();
323+
},
298324
)));
299325
assert!(
300326
(({
@@ -367,7 +393,15 @@ pub fn check_erase_3(mut key_destructor: bool, mut value_destructor: bool) {
367393
({ (*first.borrow()).clone() }),
368394
)),
369395
);
370-
(*next.borrow_mut()) = Map::erase(m.as_pointer(), &({ (*const_first.borrow()).clone() }));
396+
(*next.borrow_mut()) = Map::erase(
397+
m.as_pointer(),
398+
&({ (*const_first.borrow()).clone() }),
399+
|__key, __value| {
400+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
401+
(&&Destroy(&__value)).destroy();
402+
(&&Destroy(&__key)).destroy();
403+
},
404+
);
371405
assert!(
372406
(({
373407
let __idx0 = (1) as usize;
@@ -396,7 +430,15 @@ pub fn check_erase_3(mut key_destructor: bool, mut value_destructor: bool) {
396430
} == { (last_value).clone() })
397431
})
398432
);
399-
(*next.borrow_mut()) = Map::erase(m.as_pointer(), &({ (*last.borrow()).clone() }));
433+
(*next.borrow_mut()) = Map::erase(
434+
m.as_pointer(),
435+
&({ (*last.borrow()).clone() }),
436+
|__key, __value| {
437+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
438+
(&&Destroy(&__value)).destroy();
439+
(&&Destroy(&__key)).destroy();
440+
},
441+
);
400442
assert!(
401443
(({
402444
let __idx0 = (3) as usize;
@@ -485,6 +527,11 @@ pub fn check_erase_4(mut key_destructor: bool, mut value_destructor: bool) {
485527
Rc::new(RefCell::new(Map::erase(
486528
m.as_pointer(),
487529
&({ (*middle.borrow()).clone() }),
530+
|__key, __value| {
531+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
532+
(&&Destroy(&__value)).destroy();
533+
(&&Destroy(&__key)).destroy();
534+
},
488535
)));
489536
assert!(
490537
(({
@@ -555,7 +602,15 @@ pub fn check_erase_4(mut key_destructor: bool, mut value_destructor: bool) {
555602
({ (*first.borrow()).clone() }),
556603
)),
557604
);
558-
(*next.borrow_mut()) = Map::erase(m.as_pointer(), &({ (*const_first.borrow()).clone() }));
605+
(*next.borrow_mut()) = Map::erase(
606+
m.as_pointer(),
607+
&({ (*const_first.borrow()).clone() }),
608+
|__key, __value| {
609+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
610+
(&&Destroy(&__value)).destroy();
611+
(&&Destroy(&__key)).destroy();
612+
},
613+
);
559614
assert!(
560615
(({
561616
let __idx0 = (1) as usize;
@@ -584,7 +639,15 @@ pub fn check_erase_4(mut key_destructor: bool, mut value_destructor: bool) {
584639
} == { (last_value).clone() })
585640
})
586641
);
587-
(*next.borrow_mut()) = Map::erase(m.as_pointer(), &({ (*last.borrow()).clone() }));
642+
(*next.borrow_mut()) = Map::erase(
643+
m.as_pointer(),
644+
&({ (*last.borrow()).clone() }),
645+
|__key, __value| {
646+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
647+
(&&Destroy(&__value)).destroy();
648+
(&&Destroy(&__key)).destroy();
649+
},
650+
);
588651
assert!(
589652
(({
590653
let __idx0 = (3) as usize;
@@ -668,9 +731,16 @@ pub fn check_erase_5(mut key_destructor: bool, mut value_destructor: bool) {
668731
m.as_pointer()
669732
.with(|__m: &Map<i32, (Value<i32>, Value<i32>)>| __m.end()),
670733
));
671-
let next: Value<BidirectionalIterator<(Value<i32>, Value<i32>)>> = Rc::new(RefCell::new(
672-
Map::erase(m.as_pointer(), &({ (*middle.borrow()).clone() })),
673-
));
734+
let next: Value<BidirectionalIterator<(Value<i32>, Value<i32>)>> =
735+
Rc::new(RefCell::new(Map::erase(
736+
m.as_pointer(),
737+
&({ (*middle.borrow()).clone() }),
738+
|__key, __value| {
739+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
740+
(&&Destroy(&__value)).destroy();
741+
(&&Destroy(&__key)).destroy();
742+
},
743+
)));
674744
assert!(
675745
(({
676746
let __idx0 = (2) as usize;
@@ -741,7 +811,15 @@ pub fn check_erase_5(mut key_destructor: bool, mut value_destructor: bool) {
741811
({ (*first.borrow()).clone() }),
742812
)),
743813
);
744-
(*next.borrow_mut()) = Map::erase(m.as_pointer(), &({ (*const_first.borrow()).clone() }));
814+
(*next.borrow_mut()) = Map::erase(
815+
m.as_pointer(),
816+
&({ (*const_first.borrow()).clone() }),
817+
|__key, __value| {
818+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
819+
(&&Destroy(&__value)).destroy();
820+
(&&Destroy(&__key)).destroy();
821+
},
822+
);
745823
assert!(
746824
(({
747825
let __idx0 = (1) as usize;
@@ -770,7 +848,15 @@ pub fn check_erase_5(mut key_destructor: bool, mut value_destructor: bool) {
770848
} == { (last_value).clone() })
771849
})
772850
);
773-
(*next.borrow_mut()) = Map::erase(m.as_pointer(), &({ (*last.borrow()).clone() }));
851+
(*next.borrow_mut()) = Map::erase(
852+
m.as_pointer(),
853+
&({ (*last.borrow()).clone() }),
854+
|__key, __value| {
855+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
856+
(&&Destroy(&__value)).destroy();
857+
(&&Destroy(&__key)).destroy();
858+
},
859+
);
774860
assert!(
775861
(({
776862
let __idx0 = (3) as usize;

‎tests/unit/out/refcount/map_key_copy_ctor.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,11 @@ fn main_0() -> i32 {
200200
m.as_pointer(),
201201
&m.as_pointer()
202202
.with(|__m: &Map<Key, (Value<Key>, Value<i32>)>| __m.find(&a.as_pointer())),
203+
|__key, __value| {
204+
use libcc2rs::__swap::{Destroy, DestroyNone, DestroyWithDtor};
205+
(&&Destroy(&__value)).destroy();
206+
(&&Destroy(&__key)).destroy();
207+
},
203208
);
204209
{
205210
let rhs_0 = ((ok as i32)

0 commit comments

Comments
 (0)