Skip to content

Commit 95fb254

Browse files
authored
Evaluate default arguments lazily and support reference defaults (#459)
The old `arg.unwrap(default_value)` pattern unconditionally evaluated default_value. This is wrong if default_value has side effects and the default value is never used. Also fix default arguments bound to reference arguments.
1 parent 3f30d90 commit 95fb254

15 files changed

Lines changed: 228 additions & 30 deletions

‎cpp2rust/converter/converter.cpp‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,14 +1206,29 @@ void Converter::EmitFunctionPreamble(clang::FunctionDecl *decl) {
12061206
auto params = decl->getDefinition() ? decl->getDefinition()->parameters()
12071207
: decl->parameters();
12081208
for (auto *param : params) {
1209-
if (HasUsableDefaultArg(param)) {
1210-
auto name = GetNamedDeclAsString(param);
1211-
auto type = ToString(param->getType());
1212-
auto init = std::format("{}.unwrap_or({})", name,
1213-
ToString(param->getDefaultArg()));
1214-
StrCat(std::format("let mut {} : {} = {}", name, type, init),
1209+
if (!HasUsableDefaultArg(param)) {
1210+
continue;
1211+
}
1212+
auto name = GetNamedDeclAsString(param);
1213+
auto type = ToString(param->getType());
1214+
std::string value;
1215+
if (auto *temp = GetDefaultArgTemporary(param)) {
1216+
auto storage = std::format("__{}_default", name);
1217+
StrCat(std::format("let mut {} : Option<{}> = None", storage,
1218+
ToString(temp->getType().getUnqualifiedType())),
12151219
token::kSemiColon);
1220+
value = std::format("{}.insert({}) as {}", storage,
1221+
ToString(temp->getSubExpr()), type);
1222+
} else if (param->getType()->isReferenceType()) {
1223+
Buffer buf(*this);
1224+
ConvertVarInit(param->getType(), param->getDefaultArg());
1225+
value = std::move(buf).str();
1226+
} else {
1227+
value = ToString(param->getDefaultArg());
12161228
}
1229+
StrCat(std::format("let mut {} : {} = {}.unwrap_or_else(|| {} {{ {} }})",
1230+
name, type, name, keyword_unsafe_, value),
1231+
token::kSemiColon);
12171232
}
12181233
}
12191234

‎cpp2rust/converter/converter_lib.cpp‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,8 @@ unsigned GetCtorIndex(clang::CXXConstructorDecl *ctor) {
636636
clang::CXXConstructorDecl *
637637
GetUserDefinedDefaultConstructor(const clang::CXXRecordDecl *decl) {
638638
for (auto c : decl->ctors()) {
639-
if (c->isUserProvided() && c->isDefaultConstructor() && c->hasBody()) {
639+
if (c->isUserProvided() && c->isDefaultConstructor() && c->hasBody() &&
640+
std::ranges::all_of(c->parameters(), HasUsableDefaultArg)) {
640641
return c;
641642
}
642643
}
@@ -647,6 +648,15 @@ bool HasUsableDefaultArg(const clang::ParmVarDecl *param) {
647648
return param->hasDefaultArg() && !param->hasUninstantiatedDefaultArg();
648649
}
649650

651+
const clang::MaterializeTemporaryExpr *
652+
GetDefaultArgTemporary(const clang::ParmVarDecl *param) {
653+
if (!param->getType()->isReferenceType()) {
654+
return nullptr;
655+
}
656+
return clang::dyn_cast<clang::MaterializeTemporaryExpr>(
657+
param->getDefaultArg()->IgnoreParens());
658+
}
659+
650660
std::string GetMainFileName(const clang::ASTContext &ctx) {
651661
const auto &src_mgr = ctx.getSourceManager();
652662
auto file_id = src_mgr.getMainFileID();

‎cpp2rust/converter/converter_lib.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,9 @@ GetUserDefinedDefaultConstructor(const clang::CXXRecordDecl *decl);
154154

155155
bool HasUsableDefaultArg(const clang::ParmVarDecl *param);
156156

157+
const clang::MaterializeTemporaryExpr *
158+
GetDefaultArgTemporary(const clang::ParmVarDecl *param);
159+
157160
std::string GetMainFileName(const clang::ASTContext &ctx);
158161

159162
std::string GetFileName(const clang::Decl *decl);

‎cpp2rust/converter/models/converter_refcount.cpp‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,30 @@ void ConverterRefCount::EmitFunctionPreamble(clang::FunctionDecl *decl) {
778778
auto params = decl->getDefinition() ? decl->getDefinition()->parameters()
779779
: decl->parameters();
780780
for (auto *param : params) {
781+
if (param->getType()->isReferenceType()) {
782+
auto name = GetNamedDeclAsString(param);
783+
if (name == "_" || !HasUsableDefaultArg(param)) {
784+
continue;
785+
}
786+
auto type = ToString(param->getType());
787+
std::string value;
788+
if (auto *temp = GetDefaultArgTemporary(param)) {
789+
auto storage = std::format("__{}_default", name);
790+
StrCat(std::format("let mut {} : Option<{}> = None", storage,
791+
ToString(temp->getType().getUnqualifiedType())),
792+
token::kSemiColon);
793+
value = std::format("{}.insert(Rc::new(RefCell::new({}))).as_pointer()",
794+
storage, ToString(temp->getSubExpr()));
795+
} else {
796+
Buffer buf(*this);
797+
ConvertVarInit(param->getType(), param->getDefaultArg());
798+
value = std::move(buf).str();
799+
}
800+
StrCat(std::format("let {} : {} = {}.unwrap_or_else(|| {})", name, type,
801+
name, value),
802+
token::kSemiColon);
803+
continue;
804+
}
781805
if (!param->getType()->isReferenceType()) {
782806
auto name = GetNamedDeclAsString(param);
783807
// Skip emitting the preamble for unnamed parameters
@@ -788,7 +812,7 @@ void ConverterRefCount::EmitFunctionPreamble(clang::FunctionDecl *decl) {
788812
auto init = name;
789813

790814
if (HasUsableDefaultArg(param)) {
791-
init = std::format("{}.unwrap_or({})", name,
815+
init = std::format("{}.unwrap_or_else(|| {})", name,
792816
ToString(param->getDefaultArg()));
793817
}
794818

‎tests/unit/default_function_arguments.cpp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,23 @@ struct Bar {
99
Bar(int v = 1) : v(v) {}
1010
};
1111

12+
int counter = 0;
13+
14+
int next() { return ++counter; }
15+
16+
int lazy(int x = next()) { return x; }
17+
18+
int by_ref(const Bar &b = Bar(7)) { return b.v; }
19+
20+
Bar global_bar(9);
21+
22+
int by_global_ref(const Bar &b = global_bar) { return b.v; }
23+
24+
template <typename T> struct Holder {
25+
T v;
26+
explicit Holder(T v = 0) : v(v) {}
27+
};
28+
1229
int main() {
1330
assert(foo(1) == 11);
1431
assert(foo(1, 2) == 3);
@@ -25,5 +42,18 @@ int main() {
2542
assert(arr[0].v == 1);
2643
assert(arr[2].v == 1);
2744

45+
assert(lazy(5) == 5);
46+
assert(counter == 0);
47+
assert(lazy() == 1);
48+
assert(counter == 1);
49+
50+
assert(by_ref() == 7);
51+
assert(by_ref(Bar(3)) == 3);
52+
assert(by_global_ref() == 9);
53+
assert(by_global_ref(Bar(4)) == 4);
54+
55+
Holder<int> h(4);
56+
assert(h.v == 4);
57+
2858
return 0;
2959
}

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

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ use std::io::{Read, Seek, Write};
77
use std::os::fd::AsFd;
88
use std::rc::{Rc, Weak};
99
pub fn foo_0(mut a: i32, b: Option<i32>) -> i32 {
10-
let mut b: i32 = b.unwrap_or(10);
10+
let mut b: i32 = b.unwrap_or_else(|| 10);
1111
return (a + b);
1212
}
1313
pub fn baz_1(mut a: Ptr<i32>, b: Option<Ptr<i32>>) -> bool {
14-
let mut b: Ptr<i32> = b.unwrap_or(Ptr::<i32>::null());
14+
let mut b: Ptr<i32> = b.unwrap_or_else(|| Ptr::<i32>::null());
1515
return ({ a } == { b });
1616
}
1717
#[derive(Clone, Record, ByteRepr, VaArg, FnPtrArg)]
@@ -22,7 +22,7 @@ pub struct Bar {
2222
}
2323
impl Bar {
2424
pub fn new(v: Option<i32>) -> Self {
25-
let mut v: i32 = v.unwrap_or(1);
25+
let mut v: i32 = v.unwrap_or_else(|| 1);
2626
Self { v: v }
2727
}
2828
}
@@ -31,6 +31,43 @@ impl Default for Bar {
3131
{ Bar::new(None) }
3232
}
3333
}
34+
thread_local!(
35+
pub static counter_2: Value<i32> = Rc::new(RefCell::new(0));
36+
);
37+
pub fn next_3() -> i32 {
38+
return (*counter_2.with(Value::clone).borrow_mut()).prefix_inc();
39+
}
40+
pub fn lazy_4(x: Option<i32>) -> i32 {
41+
let mut x: i32 = x.unwrap_or_else(|| ({ next_3() }));
42+
return x;
43+
}
44+
pub fn by_ref_5(b: Option<Ptr<Bar>>) -> i32 {
45+
let mut __b_default: Option<Value<Bar>> = None;
46+
let b: Ptr<Bar> = b.unwrap_or_else(|| {
47+
__b_default
48+
.insert(Rc::new(RefCell::new(Bar::new({ Some(7) }))))
49+
.as_pointer()
50+
});
51+
return b.with(|__s| __s.v);
52+
}
53+
thread_local!(
54+
pub static global_bar_6: Value<Bar> = Rc::new(RefCell::new(Bar::new({ Some(9) })));
55+
);
56+
pub fn by_global_ref_7(b: Option<Ptr<Bar>>) -> i32 {
57+
let b: Ptr<Bar> = b.unwrap_or_else(|| global_bar_6.with(|v| v.as_pointer()));
58+
return b.with(|__s| __s.v);
59+
}
60+
#[derive(Clone, Record, ByteRepr, VaArg, FnPtrArg, Default)]
61+
#[byte_size(4)]
62+
pub struct Holder_int_ {
63+
#[offset(0)]
64+
pub v: i32,
65+
}
66+
impl Holder_int_ {
67+
pub fn new(mut v: i32) -> Self {
68+
Self { v: v }
69+
}
70+
}
3471
pub fn main() {
3572
__cpp2rust_init_globals();
3673
std::process::exit(main_0());
@@ -54,6 +91,29 @@ fn main_0() -> i32 {
5491
let mut arr: [Bar; 3] = [Bar::new(None), Bar::new(None), Bar::new(None)];
5592
assert!(({ arr[(0) as usize].v } == 1));
5693
assert!(({ arr[(2) as usize].v } == 1));
94+
assert!((({ lazy_4(Some(5),) }) == 5));
95+
assert!((counter_2.with(|rc| *rc.borrow()) == 0));
96+
assert!((({ lazy_4(None,) }) == 1));
97+
assert!((counter_2.with(|rc| *rc.borrow()) == 1));
98+
assert!((({ by_ref_5(None,) }) == 7));
99+
assert!(
100+
(({
101+
let _b: Value<Bar> = Rc::new(RefCell::new(Bar::new({ Some(3) })));
102+
by_ref_5(Some(_b.as_pointer()))
103+
}) == 3)
104+
);
105+
assert!((({ by_global_ref_7(None,) }) == 9));
106+
assert!(
107+
(({
108+
let _b: Value<Bar> = Rc::new(RefCell::new(Bar::new({ Some(4) })));
109+
by_global_ref_7(Some(_b.as_pointer()))
110+
}) == 4)
111+
);
112+
let mut h: Holder_int_ = Holder_int_::new({ 4 });
113+
assert!((h.v == 4));
57114
return 0;
58115
}
59-
pub fn __cpp2rust_init_globals() {}
116+
pub fn __cpp2rust_init_globals() {
117+
let _ = counter_2.with(|_| ());
118+
let _ = global_bar_6.with(|_| ());
119+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ pub fn identity_0(mut x: i32) -> i32 {
1010
return x;
1111
}
1212
pub fn apply_1(mut x: i32, fn_: Option<FnPtr<fn(i32) -> i32>>) -> i32 {
13-
let mut fn_: FnPtr<fn(i32) -> i32> = fn_.unwrap_or(FnPtr::<fn(i32) -> i32>::null());
13+
let mut fn_: FnPtr<fn(i32) -> i32> = fn_.unwrap_or_else(|| FnPtr::<fn(i32) -> i32>::null());
1414
if !(fn_).is_null() {
1515
return ({ fn_.call(x) });
1616
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@ use std::io::{Read, Seek, Write};
77
use std::os::fd::AsFd;
88
use std::rc::{Rc, Weak};
99
pub fn out_param_0(out: Option<Ptr<usize>>) {
10-
let mut out: Ptr<usize> = out.unwrap_or(Ptr::<usize>::null());
10+
let mut out: Ptr<usize> = out.unwrap_or_else(|| Ptr::<usize>::null());
1111
if !(out).is_null() {
1212
out.write(4_usize);
1313
}
1414
}
1515
pub fn parse_1(mut v: i32, idx: Option<Ptr<usize>>) -> i32 {
16-
let mut idx: Ptr<usize> = idx.unwrap_or(Ptr::<usize>::null());
16+
let mut idx: Ptr<usize> = idx.unwrap_or_else(|| Ptr::<usize>::null());
1717
if !(idx).is_null() {
1818
idx.write(3_usize);
1919
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,15 @@ impl NoDefault {
5353
}
5454
}
5555
pub fn used_0(x: Option<A>) -> i32 {
56-
let mut x: A = x.unwrap_or(A::new_1());
56+
let mut x: A = x.unwrap_or_else(|| A::new_1());
5757
return x.v;
5858
}
5959
pub fn used_1(x: Option<B>) -> i32 {
60-
let mut x: B = x.unwrap_or(B::new());
60+
let mut x: B = x.unwrap_or_else(|| B::new());
6161
return x.v;
6262
}
6363
pub fn scaled_2(mut x: A, n: Option<i32>) -> i32 {
64-
let mut n: i32 = n.unwrap_or((4usize as i32));
64+
let mut n: i32 = n.unwrap_or_else(|| (4usize as i32));
6565
return (x.v * n);
6666
}
6767
pub fn always_given_3(mut x: NoDefault) -> i32 {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ pub fn sum_3(mut arr: Ptr<i32>, mut n: i32) -> i32 {
3838
return s;
3939
}
4040
pub fn countdown_4(mut n: i32, step: Option<i32>) -> i32 {
41-
let mut step: i32 = step.unwrap_or(1);
41+
let mut step: i32 = step.unwrap_or_else(|| 1);
4242
let mut steps: i32 = 0;
4343
'loop_: while (n > 0) {
4444
n -= step;

0 commit comments

Comments
 (0)