Skip to content

Commit de0dfa2

Browse files
nunoplopesnunoplopes
committed
Don't clone whole global arrays on a read
1 parent cea98f6 commit de0dfa2

19 files changed

Lines changed: 193 additions & 99 deletions

‎cpp2rust/converter/models/converter_refcount.cpp‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -914,10 +914,13 @@ bool ConverterRefCount::VisitDeclRefExpr(clang::DeclRefExpr *expr) {
914914

915915
bool fresh = false;
916916
if (isRValue()) {
917-
if (is_global_value) {
918-
StrCat(str, TypeIsCopyable(decl_t) ? ".with(|rc| *rc.borrow())"
919-
: ".with(|rc| rc.borrow().clone())");
917+
if (is_global_value && TypeIsCopyable(decl_t)) {
918+
StrCat(str, ".with(|rc| *rc.borrow())");
920919
fresh = true;
920+
} else if (is_global_value) {
921+
// Borrow the global in place, like a local. Cloning it would be
922+
// expensive for arrays, and writes through its fields would be lost.
923+
StrCat(std::format("(*{}.with(Value::clone).borrow())", std::move(str)));
921924
} else if (is_global_ptr) {
922925
StrCat(str);
923926
fresh = true;
@@ -2504,6 +2507,26 @@ std::string ConverterRefCount::ConvertSubscriptIndex(clang::Expr *idx) {
25042507
return str;
25052508
}
25062509

2510+
clang::DeclRefExpr *ConverterRefCount::GetGlobalArrayRValue(clang::Expr *base) {
2511+
auto *ref = clang::dyn_cast<clang::DeclRefExpr>(base->IgnoreImplicit());
2512+
if (!isRValue() || !ref || !IsGlobalVar(ref) ||
2513+
ref->getDecl()->getType()->isReferenceType() ||
2514+
(ShouldReplaceWithMappedBody(ref) && !GetMappedAsString(ref).empty())) {
2515+
return nullptr;
2516+
}
2517+
auto *arr_ty = ctx_.getAsArrayType(ref->getType());
2518+
if (!arr_ty) {
2519+
return nullptr;
2520+
}
2521+
// Records are accessed in place, as their fields may be written to.
2522+
auto elem_ty = arr_ty->getElementType();
2523+
if (!TypeIsCopyable(elem_ty) && !elem_ty->isPointerType() &&
2524+
!elem_ty->isArrayType()) {
2525+
return nullptr;
2526+
}
2527+
return ref;
2528+
}
2529+
25072530
void ConverterRefCount::ConvertArraySubscript(clang::Expr *base,
25082531
clang::Expr *idx,
25092532
clang::QualType type) {
@@ -2539,6 +2562,14 @@ void ConverterRefCount::ConvertArraySubscript(clang::Expr *base,
25392562
}
25402563

25412564
computed_expr_type_ = ComputedExprType::FreshPointer;
2565+
} else if (auto *global = GetGlobalArrayRValue(base)) {
2566+
// Read just the element instead of cloning the whole global array.
2567+
// The index is evaluated first, as it may itself access the global.
2568+
StrCat(std::format(
2569+
"({{ let __idx = ({}) as usize; {}.with(|rc| rc.borrow()[__idx]{}) }})",
2570+
ConvertRValue(idx), ConvertDeclRefExpr(global),
2571+
TypeIsCopyable(type) ? "" : ".clone()"));
2572+
SetFreshType(type);
25422573
} else {
25432574
if (isLValue() &&
25442575
clang::isa<clang::ArraySubscriptExpr>(base->IgnoreImplicit())) {

‎cpp2rust/converter/models/converter_refcount.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,9 @@ class ConverterRefCount final : public Converter {
219219

220220
void ConvertArraySubscript(clang::Expr *base, clang::Expr *idx,
221221
clang::QualType type) override;
222+
// The global array referenced by base when one of its elements is read as
223+
// an rvalue and can be copied out of the array, or nullptr otherwise.
224+
clang::DeclRefExpr *GetGlobalArrayRValue(clang::Expr *base);
222225
void ConvertPointerSubscript(clang::ArraySubscriptExpr *expr) override;
223226

224227
void ConvertFunctionMain(const clang::FunctionDecl *decl,

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

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -90,23 +90,29 @@ pub fn main() {
9090
}
9191
fn main_0() -> i32 {
9292
assert!(
93-
((*(*items_3.with(|rc| rc.borrow().clone())[(0) as usize]
94-
.upgrade()
95-
.deref())
93+
((*(*({
94+
let __idx = (0) as usize;
95+
items_3.with(|rc| rc.borrow()[__idx].clone())
96+
})
97+
.upgrade()
98+
.deref())
9699
.value
97100
.borrow())
98101
== 1)
99102
);
100103
assert!(
101-
((*(*items_3.with(|rc| rc.borrow().clone())[(1) as usize]
102-
.upgrade()
103-
.deref())
104+
((*(*({
105+
let __idx = (1) as usize;
106+
items_3.with(|rc| rc.borrow()[__idx].clone())
107+
})
108+
.upgrade()
109+
.deref())
104110
.value
105111
.borrow())
106112
== 2)
107113
);
108114
assert!(
109-
((*(*(*obj_4.with(|rc| rc.borrow().clone()).p.borrow())
115+
((*(*(*(*obj_4.with(Value::clone).borrow()).p.borrow())
110116
.upgrade()
111117
.deref())
112118
.value
@@ -120,17 +126,23 @@ fn main_0() -> i32 {
120126
])));
121127
);
122128
assert!(
123-
((*(*cache_5.with(|rc| rc.borrow().clone())[(0) as usize]
124-
.upgrade()
125-
.deref())
129+
((*(*({
130+
let __idx = (0) as usize;
131+
cache_5.with(|rc| rc.borrow()[__idx].clone())
132+
})
133+
.upgrade()
134+
.deref())
126135
.value
127136
.borrow())
128137
== 1)
129138
);
130139
assert!(
131-
((*(*cache_5.with(|rc| rc.borrow().clone())[(1) as usize]
132-
.upgrade()
133-
.deref())
140+
((*(*({
141+
let __idx = (1) as usize;
142+
cache_5.with(|rc| rc.borrow()[__idx].clone())
143+
})
144+
.upgrade()
145+
.deref())
134146
.value
135147
.borrow())
136148
== 2)

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,11 @@ pub fn main() {
5959
std::process::exit(main_0());
6060
}
6161
fn main_0() -> i32 {
62-
assert!(((((*s_0.with(|rc| rc.borrow().clone()).head.borrow()) == 5) as i32) != 0));
62+
assert!(((((*(*s_0.with(Value::clone).borrow()).head.borrow()) == 5) as i32) != 0));
6363
let i: Value<i32> = Rc::new(RefCell::new(0));
6464
'loop_: while ((((*i.borrow()) < 3) as i32) != 0) {
6565
assert!(
66-
((((*s_0.with(|rc| rc.borrow().clone()).tail.borrow())[(*i.borrow()) as usize] == 0)
66+
((((*(*s_0.with(Value::clone).borrow()).tail.borrow())[(*i.borrow()) as usize] == 0)
6767
as i32)
6868
!= 0)
6969
);
@@ -72,7 +72,7 @@ fn main_0() -> i32 {
7272
let i: Value<i32> = Rc::new(RefCell::new(0));
7373
'loop_: while ((((*i.borrow()) < 4) as i32) != 0) {
7474
assert!(
75-
(((((*s_0.with(|rc| rc.borrow().clone()).buf.borrow())[(*i.borrow()) as usize] as i32)
75+
(((((*(*s_0.with(Value::clone).borrow()).buf.borrow())[(*i.borrow()) as usize] as i32)
7676
== 0) as i32)
7777
!= 0)
7878
);

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

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -219,66 +219,66 @@ pub fn check_local_static_5() {
219219
thread_local!(
220220
static local_p_8: Value<Ptr<i32>> = Rc::new(RefCell::new(Ptr::<i32>::null()));
221221
);
222-
assert!((*local_outer_6.with(|rc| rc.borrow().clone()).p1.borrow()).is_null());
223-
assert!((*local_outer_6.with(|rc| rc.borrow().clone()).fn_.borrow()).is_null());
224-
assert!((local_fn_7.with(|rc| rc.borrow().clone())).is_null());
225-
assert!((local_p_8.with(|rc| rc.borrow().clone())).is_null());
222+
assert!((*(*local_outer_6.with(Value::clone).borrow()).p1.borrow()).is_null());
223+
assert!((*(*local_outer_6.with(Value::clone).borrow()).fn_.borrow()).is_null());
224+
assert!((*local_fn_7.with(Value::clone).borrow()).is_null());
225+
assert!((*local_p_8.with(Value::clone).borrow()).is_null());
226226
}
227227
pub fn main() {
228228
__cpp2rust_init_globals();
229229
std::process::exit(main_0());
230230
}
231231
fn main_0() -> i32 {
232-
assert!((static_fn_0.with(|rc| rc.borrow().clone())).is_null());
233-
assert!((*static_outer_1.with(|rc| rc.borrow().clone()).p1.borrow()).is_null());
234-
assert!((*static_outer_1.with(|rc| rc.borrow().clone()).p2.borrow()).is_null());
235-
assert!((*static_outer_1.with(|rc| rc.borrow().clone()).cp.borrow()).is_null());
236-
assert!((*static_outer_1.with(|rc| rc.borrow().clone()).pp.borrow()).is_null());
237-
assert!((*static_outer_1.with(|rc| rc.borrow().clone()).fn_.borrow()).is_null());
232+
assert!((*static_fn_0.with(Value::clone).borrow()).is_null());
233+
assert!((*(*static_outer_1.with(Value::clone).borrow()).p1.borrow()).is_null());
234+
assert!((*(*static_outer_1.with(Value::clone).borrow()).p2.borrow()).is_null());
235+
assert!((*(*static_outer_1.with(Value::clone).borrow()).cp.borrow()).is_null());
236+
assert!((*(*static_outer_1.with(Value::clone).borrow()).pp.borrow()).is_null());
237+
assert!((*(*static_outer_1.with(Value::clone).borrow()).fn_.borrow()).is_null());
238238
let i: Value<i32> = Rc::new(RefCell::new(0));
239239
'loop_: while ((*i.borrow()) < 3) {
240240
assert!(
241-
((*static_outer_1.with(|rc| rc.borrow().clone()).arr.borrow())[(*i.borrow()) as usize])
241+
((*(*static_outer_1.with(Value::clone).borrow()).arr.borrow())[(*i.borrow()) as usize])
242242
.is_null()
243243
);
244244
(*i.borrow_mut()).prefix_inc();
245245
}
246246
assert!(
247-
(*(*static_outer_1.with(|rc| rc.borrow().clone()).inner.borrow())
247+
(*(*(*static_outer_1.with(Value::clone).borrow()).inner.borrow())
248248
.name
249249
.borrow())
250250
.is_null()
251251
);
252252
let i: Value<i32> = Rc::new(RefCell::new(0));
253253
'loop_: while ((*i.borrow()) < 2) {
254254
assert!(
255-
(*static_inner_array_2.with(|rc| rc.borrow().clone())[(*i.borrow()) as usize]
255+
(*(*static_inner_array_2.with(Value::clone).borrow())[(*i.borrow()) as usize]
256256
.name
257257
.borrow())
258258
.is_null()
259259
);
260260
(*i.borrow_mut()).prefix_inc();
261261
}
262-
assert!((*static_foo_3.with(|rc| rc.borrow().clone()).s2.borrow()).is_null());
263-
assert!((*static_foo_3.with(|rc| rc.borrow().clone()).fn1.borrow()).is_null());
264-
assert!((*static_foo_3.with(|rc| rc.borrow().clone()).fn2.borrow()).is_null());
265-
assert!(((*static_foo_3.with(|rc| rc.borrow().clone()).n.borrow()) == 42));
262+
assert!((*(*static_foo_3.with(Value::clone).borrow()).s2.borrow()).is_null());
263+
assert!((*(*static_foo_3.with(Value::clone).borrow()).fn1.borrow()).is_null());
264+
assert!((*(*static_foo_3.with(Value::clone).borrow()).fn2.borrow()).is_null());
265+
assert!(((*(*static_foo_3.with(Value::clone).borrow()).n.borrow()) == 42));
266266
let i: Value<i32> = Rc::new(RefCell::new(0));
267267
'loop_: while ((*i.borrow()) < 2) {
268268
assert!(
269-
(*static_foo_array_4.with(|rc| rc.borrow().clone())[(*i.borrow()) as usize]
269+
(*(*static_foo_array_4.with(Value::clone).borrow())[(*i.borrow()) as usize]
270270
.s2
271271
.borrow())
272272
.is_null()
273273
);
274274
assert!(
275-
(*static_foo_array_4.with(|rc| rc.borrow().clone())[(*i.borrow()) as usize]
275+
(*(*static_foo_array_4.with(Value::clone).borrow())[(*i.borrow()) as usize]
276276
.fn1
277277
.borrow())
278278
.is_null()
279279
);
280280
assert!(
281-
(*static_foo_array_4.with(|rc| rc.borrow().clone())[(*i.borrow()) as usize]
281+
(*(*static_foo_array_4.with(Value::clone).borrow())[(*i.borrow()) as usize]
282282
.fn2
283283
.borrow())
284284
.is_null()

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,24 @@ fn main_0() -> i32 {
374374
let _dtor_o = ScopedDestructor::new(&o, |__p| __p.destructor());
375375
}
376376
assert!((order_count_2.with(|rc| *rc.borrow()) == 3));
377-
assert!((order_1.with(|rc| rc.borrow().clone())[(0) as usize] == 3));
378-
assert!((order_1.with(|rc| rc.borrow().clone())[(1) as usize] == 2));
379-
assert!((order_1.with(|rc| rc.borrow().clone())[(2) as usize] == 1));
377+
assert!(
378+
(({
379+
let __idx = (0) as usize;
380+
order_1.with(|rc| rc.borrow()[__idx])
381+
}) == 3)
382+
);
383+
assert!(
384+
(({
385+
let __idx = (1) as usize;
386+
order_1.with(|rc| rc.borrow()[__idx])
387+
}) == 2)
388+
);
389+
assert!(
390+
(({
391+
let __idx = (2) as usize;
392+
order_1.with(|rc| rc.borrow()[__idx])
393+
}) == 1)
394+
);
380395
return 0;
381396
}
382397
pub trait ArrayMemberImpl {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ pub fn main() {
4646
std::process::exit(main_0());
4747
}
4848
fn main_0() -> i32 {
49-
assert!(((((*config_0.with(|rc| rc.borrow().clone()).count.borrow()) == 0) as i32) != 0));
49+
assert!(((((*(*config_0.with(Value::clone).borrow()).count.borrow()) == 0) as i32) != 0));
5050
assert!(
51-
(((((*config_0.with(|rc| rc.borrow().clone()).mode.borrow()) as u32)
51+
(((((*(*config_0.with(Value::clone).borrow()).mode.borrow()) as u32)
5252
== ((Mode_MODE_NONE as i32) as u32)) as i32)
5353
!= 0)
5454
);

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,37 +198,37 @@ fn main_0() -> i32 {
198198
assert!(((global_opt_1.with(|rc| *rc.borrow()) as i32) == (Option_OPT_B as i32)));
199199
assert!(((global_tag_2.with(|rc| *rc.borrow()) as i32) == (Tag_TAG_TWO as i32)));
200200
assert!(
201-
(((*entries_3.with(|rc| rc.borrow().clone())[(0) as usize]
201+
(((*(*entries_3.with(Value::clone).borrow())[(0) as usize]
202202
.color
203203
.borrow()) as i32)
204204
== (Color_RED as i32))
205205
);
206206
assert!(
207-
(((*entries_3.with(|rc| rc.borrow().clone())[(0) as usize]
207+
(((*(*entries_3.with(Value::clone).borrow())[(0) as usize]
208208
.opt
209209
.borrow()) as i32)
210210
== (Option_OPT_NONE as i32))
211211
);
212212
assert!(
213-
(((*entries_3.with(|rc| rc.borrow().clone())[(1) as usize]
213+
(((*(*entries_3.with(Value::clone).borrow())[(1) as usize]
214214
.color
215215
.borrow()) as i32)
216216
== (Color_GREEN as i32))
217217
);
218218
assert!(
219-
(((*entries_3.with(|rc| rc.borrow().clone())[(1) as usize]
219+
(((*(*entries_3.with(Value::clone).borrow())[(1) as usize]
220220
.opt
221221
.borrow()) as i32)
222222
== (Option_OPT_A as i32))
223223
);
224224
assert!(
225-
(((*entries_3.with(|rc| rc.borrow().clone())[(2) as usize]
225+
(((*(*entries_3.with(Value::clone).borrow())[(2) as usize]
226226
.color
227227
.borrow()) as i32)
228228
== (Color_BLUE as i32))
229229
);
230230
assert!(
231-
(((*entries_3.with(|rc| rc.borrow().clone())[(2) as usize]
231+
(((*(*entries_3.with(Value::clone).borrow())[(2) as usize]
232232
.opt
233233
.borrow()) as i32)
234234
== (Option_OPT_C as i32))

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -212,42 +212,42 @@ fn main_0() -> i32 {
212212
!= 0)
213213
);
214214
assert!(
215-
(((((*entries_3.with(|rc| rc.borrow().clone())[(0) as usize]
215+
(((((*(*entries_3.with(Value::clone).borrow())[(0) as usize]
216216
.color
217217
.borrow()) as u32)
218218
== ((Color_RED as i32) as u32)) as i32)
219219
!= 0)
220220
);
221221
assert!(
222-
(((((*entries_3.with(|rc| rc.borrow().clone())[(0) as usize]
222+
(((((*(*entries_3.with(Value::clone).borrow())[(0) as usize]
223223
.opt
224224
.borrow()) as u32)
225225
== ((Option_OPT_NONE as i32) as u32)) as i32)
226226
!= 0)
227227
);
228228
assert!(
229-
(((((*entries_3.with(|rc| rc.borrow().clone())[(1) as usize]
229+
(((((*(*entries_3.with(Value::clone).borrow())[(1) as usize]
230230
.color
231231
.borrow()) as u32)
232232
== ((Color_GREEN as i32) as u32)) as i32)
233233
!= 0)
234234
);
235235
assert!(
236-
(((((*entries_3.with(|rc| rc.borrow().clone())[(1) as usize]
236+
(((((*(*entries_3.with(Value::clone).borrow())[(1) as usize]
237237
.opt
238238
.borrow()) as u32)
239239
== ((Option_OPT_A as i32) as u32)) as i32)
240240
!= 0)
241241
);
242242
assert!(
243-
(((((*entries_3.with(|rc| rc.borrow().clone())[(2) as usize]
243+
(((((*(*entries_3.with(Value::clone).borrow())[(2) as usize]
244244
.color
245245
.borrow()) as u32)
246246
== ((Color_BLUE as i32) as u32)) as i32)
247247
!= 0)
248248
);
249249
assert!(
250-
(((((*entries_3.with(|rc| rc.borrow().clone())[(2) as usize]
250+
(((((*(*entries_3.with(Value::clone).borrow())[(2) as usize]
251251
.opt
252252
.borrow()) as u32)
253253
== ((Option_OPT_C as i32) as u32)) as i32)
@@ -267,7 +267,7 @@ fn main_0() -> i32 {
267267
!= 0)
268268
);
269269
assert!(
270-
(((((*entries_3.with(|rc| rc.borrow().clone())[(*idx.borrow()) as usize]
270+
(((((*(*entries_3.with(Value::clone).borrow())[(*idx.borrow()) as usize]
271271
.opt
272272
.borrow()) as u32)
273273
== ((Option_OPT_A as i32) as u32)) as i32)

0 commit comments

Comments
 (0)