Skip to content

Commit e71c387

Browse files
authored
Fix comparison operator call for move-only structs (#355)
A move-only type, i.e. one that has copy constructor and assignment deleted, cannot be `.clone()`'ed. PartialOrd/Ord assumed that the type is aways clonable. This PR fixes PartialOrd/Ord by cloning the Rc's of the fields, instead of calling clone on the entire struct. The comparison semantics are not changed.
1 parent a79e315 commit e71c387

11 files changed

Lines changed: 318 additions & 62 deletions

‎cpp2rust/converter/models/converter_refcount.cpp‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -438,10 +438,9 @@ bool ConverterRefCount::VisitOffsetOfExpr(clang::OffsetOfExpr *expr) {
438438
std::string ConverterRefCount::GetComparisonCall(
439439
const clang::FunctionDecl *op, const clang::CXXRecordDecl *decl,
440440
std::string_view lhs, std::string_view rhs) {
441-
auto lhs_ptr =
442-
std::format("Rc::new(RefCell::new({}.clone())).as_pointer()", lhs);
443-
auto rhs_ptr =
444-
std::format("Rc::new(RefCell::new({}.clone())).as_pointer()", rhs);
441+
PushConversionKind push(*this, ConversionKind::FullRefCount);
442+
auto lhs_ptr = BoxValue(GetShallowCopy(decl, lhs)) + ".as_pointer()";
443+
auto rhs_ptr = BoxValue(GetShallowCopy(decl, rhs)) + ".as_pointer()";
445444
if (const auto *method = clang::dyn_cast<clang::CXXMethodDecl>(op)) {
446445
return std::format("{}::{}(&{}, {})", GetUFCSName(method),
447446
GetMethodName(method), lhs_ptr, rhs_ptr);
@@ -450,17 +449,14 @@ std::string ConverterRefCount::GetComparisonCall(
450449
lhs_ptr, rhs_ptr);
451450
}
452451

453-
void ConverterRefCount::EmitShallowCopy(const clang::RecordDecl *decl) {
454-
StrCat("Rc::new");
455-
PushParen rc_paren(*this);
456-
StrCat("RefCell::new");
457-
PushParen cell_paren(*this);
458-
StrCat(GetRecordName(decl));
459-
PushBrace init_brace(*this);
452+
std::string ConverterRefCount::GetShallowCopy(const clang::RecordDecl *decl,
453+
std::string_view src) {
454+
std::string fields;
460455
for (auto *field : decl->fields()) {
461456
auto name = GetNamedDeclAsString(field);
462-
StrCat(std::format("{0}: self.{0}.clone(),", name));
457+
fields += std::format("{0}: {1}.{0}.clone(),", name, src);
463458
}
459+
return std::format("{} {{ {} }}", GetRecordName(decl), fields);
464460
}
465461

466462
void ConverterRefCount::AddCloneTrait(const clang::RecordDecl *decl) {
@@ -500,9 +496,9 @@ void ConverterRefCount::AddCloneTrait(const clang::RecordDecl *decl) {
500496
StrCat("fn clone(&self) -> Self {");
501497

502498
if (auto *ctor = GetUserDefinedCopyConstructor(cxx)) {
503-
StrCat(std::format("let __src: Value<{}> =", record_name));
504-
EmitShallowCopy(decl);
505-
StrCat(token::kSemiColon);
499+
PushConversionKind push(*this, ConversionKind::FullRefCount);
500+
StrCat(std::format("let __src: Value<{}> = {};", record_name,
501+
BoxValue(GetShallowCopy(decl, "self"))));
506502
StrCat(std::format("{}::{}(__src.as_pointer())", record_name,
507503
GetCtorName(ctor)));
508504
} else {

‎cpp2rust/converter/models/converter_refcount.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ class ConverterRefCount final : public Converter {
4141
std::string_view lhs,
4242
std::string_view rhs) override;
4343

44-
void EmitShallowCopy(const clang::RecordDecl *decl);
44+
std::string GetShallowCopy(const clang::RecordDecl *decl,
45+
std::string_view src);
4546
void AddCloneTrait(const clang::RecordDecl *decl) override;
4647

4748
void AddByteReprTrait(const clang::RecordDecl *decl) override;
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#include <cassert>
2+
3+
class S {
4+
int data_;
5+
6+
public:
7+
S(int data) : data_(data) {}
8+
S(const S &) = delete;
9+
S &operator=(const S &) = delete;
10+
S(S &&) = default;
11+
12+
friend bool operator==(const S &x, const S &y) { return x.data_ == y.data_; }
13+
friend bool operator<(const S &x, const S &y) { return x.data_ < y.data_; }
14+
};
15+
16+
int main() {
17+
S a(1), b(2), c(1);
18+
assert(a == c);
19+
assert(a < b);
20+
assert(!(b < a));
21+
return 0;
22+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ impl std::cmp::PartialEq for V {
3737
fn eq(&self, other: &Self) -> bool {
3838
{
3939
operator_eq_1(
40-
Rc::new(RefCell::new(self.clone())).as_pointer(),
41-
Rc::new(RefCell::new(other.clone())).as_pointer(),
40+
Rc::new(RefCell::new(V { x: self.x.clone() })).as_pointer(),
41+
Rc::new(RefCell::new(V { x: other.x.clone() })).as_pointer(),
4242
)
4343
}
4444
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ impl std::cmp::Ord for S {
1414
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
1515
{
1616
if operator_lt_0(
17-
Rc::new(RefCell::new(self.clone())).as_pointer(),
18-
Rc::new(RefCell::new(other.clone())).as_pointer(),
17+
Rc::new(RefCell::new(S { v: self.v.clone() })).as_pointer(),
18+
Rc::new(RefCell::new(S { v: other.v.clone() })).as_pointer(),
1919
) {
2020
std::cmp::Ordering::Less
2121
} else if operator_lt_0(
22-
Rc::new(RefCell::new(other.clone())).as_pointer(),
23-
Rc::new(RefCell::new(self.clone())).as_pointer(),
22+
Rc::new(RefCell::new(S { v: other.v.clone() })).as_pointer(),
23+
Rc::new(RefCell::new(S { v: self.v.clone() })).as_pointer(),
2424
) {
2525
std::cmp::Ordering::Greater
2626
} else {
@@ -38,8 +38,8 @@ impl std::cmp::PartialEq for S {
3838
fn eq(&self, other: &Self) -> bool {
3939
{
4040
operator_eq_1(
41-
Rc::new(RefCell::new(self.clone())).as_pointer(),
42-
Rc::new(RefCell::new(other.clone())).as_pointer(),
41+
Rc::new(RefCell::new(S { v: self.v.clone() })).as_pointer(),
42+
Rc::new(RefCell::new(S { v: other.v.clone() })).as_pointer(),
4343
)
4444
}
4545
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ impl std::cmp::Ord for S {
1414
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
1515
{
1616
if SImpl::operator_lt_pconstS_const(
17-
&Rc::new(RefCell::new(self.clone())).as_pointer(),
18-
Rc::new(RefCell::new(other.clone())).as_pointer(),
17+
&Rc::new(RefCell::new(S { v: self.v.clone() })).as_pointer(),
18+
Rc::new(RefCell::new(S { v: other.v.clone() })).as_pointer(),
1919
) {
2020
std::cmp::Ordering::Less
2121
} else if SImpl::operator_lt_pconstS_const(
22-
&Rc::new(RefCell::new(other.clone())).as_pointer(),
23-
Rc::new(RefCell::new(self.clone())).as_pointer(),
22+
&Rc::new(RefCell::new(S { v: other.v.clone() })).as_pointer(),
23+
Rc::new(RefCell::new(S { v: self.v.clone() })).as_pointer(),
2424
) {
2525
std::cmp::Ordering::Greater
2626
} else {
@@ -38,8 +38,8 @@ impl std::cmp::PartialEq for S {
3838
fn eq(&self, other: &Self) -> bool {
3939
{
4040
SImpl::operator_eq(
41-
&Rc::new(RefCell::new(self.clone())).as_pointer(),
42-
Rc::new(RefCell::new(other.clone())).as_pointer(),
41+
&Rc::new(RefCell::new(S { v: self.v.clone() })).as_pointer(),
42+
Rc::new(RefCell::new(S { v: other.v.clone() })).as_pointer(),
4343
)
4444
}
4545
}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
extern crate libcc2rs;
2+
use libcc2rs::*;
3+
use std::cell::RefCell;
4+
use std::collections::BTreeMap;
5+
use std::io::prelude::*;
6+
use std::io::{Read, Seek, Write};
7+
use std::os::fd::AsFd;
8+
use std::rc::{Rc, Weak};
9+
pub fn operator_eq_0(x: Ptr<S>, y: Ptr<S>) -> bool {
10+
return {
11+
let _lhs = (*(*x.upgrade().deref()).data_.borrow());
12+
_lhs == (*(*y.upgrade().deref()).data_.borrow())
13+
};
14+
}
15+
pub fn operator_lt_1(x: Ptr<S>, y: Ptr<S>) -> bool {
16+
return {
17+
let _lhs = (*(*x.upgrade().deref()).data_.borrow());
18+
_lhs < (*(*y.upgrade().deref()).data_.borrow())
19+
};
20+
}
21+
#[derive(Default)]
22+
pub struct S {
23+
data_: Value<i32>,
24+
}
25+
impl S {
26+
pub fn S(data: i32) -> Self {
27+
let data: Value<i32> = Rc::new(RefCell::new(data));
28+
let __this: Value<S> = Rc::new(RefCell::new(Self {
29+
data_: Rc::new(RefCell::new((*data.borrow()))),
30+
}));
31+
let this: Ptr<S> = __this.as_pointer();
32+
Rc::try_unwrap(__this).ok().unwrap().into_inner()
33+
}
34+
}
35+
impl std::cmp::Ord for S {
36+
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
37+
{
38+
if operator_lt_1(
39+
Rc::new(RefCell::new(S {
40+
data_: self.data_.clone(),
41+
}))
42+
.as_pointer(),
43+
Rc::new(RefCell::new(S {
44+
data_: other.data_.clone(),
45+
}))
46+
.as_pointer(),
47+
) {
48+
std::cmp::Ordering::Less
49+
} else if operator_lt_1(
50+
Rc::new(RefCell::new(S {
51+
data_: other.data_.clone(),
52+
}))
53+
.as_pointer(),
54+
Rc::new(RefCell::new(S {
55+
data_: self.data_.clone(),
56+
}))
57+
.as_pointer(),
58+
) {
59+
std::cmp::Ordering::Greater
60+
} else {
61+
std::cmp::Ordering::Equal
62+
}
63+
}
64+
}
65+
}
66+
impl std::cmp::PartialOrd for S {
67+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
68+
Some(self.cmp(other))
69+
}
70+
}
71+
impl std::cmp::PartialEq for S {
72+
fn eq(&self, other: &Self) -> bool {
73+
{
74+
operator_eq_0(
75+
Rc::new(RefCell::new(S {
76+
data_: self.data_.clone(),
77+
}))
78+
.as_pointer(),
79+
Rc::new(RefCell::new(S {
80+
data_: other.data_.clone(),
81+
}))
82+
.as_pointer(),
83+
)
84+
}
85+
}
86+
}
87+
impl std::cmp::Eq for S {}
88+
impl ByteRepr for S {
89+
fn byte_size() -> usize {
90+
4
91+
}
92+
fn to_bytes(&self, buf: &mut [u8]) {
93+
(*self.data_.borrow()).to_bytes(&mut buf[0..4]);
94+
}
95+
fn from_bytes(buf: &[u8]) -> Self {
96+
Self {
97+
data_: Rc::new(RefCell::new(<i32>::from_bytes(&buf[0..4]))),
98+
}
99+
}
100+
}
101+
pub fn main() {
102+
std::process::exit(main_0());
103+
}
104+
fn main_0() -> i32 {
105+
let a: Value<S> = Rc::new(RefCell::new(S::S({ 1 })));
106+
let b: Value<S> = Rc::new(RefCell::new(S::S({ 2 })));
107+
let c: Value<S> = Rc::new(RefCell::new(S::S({ 1 })));
108+
assert!(
109+
({
110+
let _x: Ptr<S> = a.as_pointer();
111+
operator_eq_0(_x, c.as_pointer())
112+
})
113+
);
114+
assert!(
115+
({
116+
let _x: Ptr<S> = a.as_pointer();
117+
operator_lt_1(_x, b.as_pointer())
118+
})
119+
);
120+
assert!(
121+
!({
122+
let _x: Ptr<S> = b.as_pointer();
123+
operator_lt_1(_x, a.as_pointer())
124+
})
125+
);
126+
return 0;
127+
}

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

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,29 @@ impl std::cmp::Ord for Pair {
1515
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
1616
{
1717
if PairImpl::operator_lt(
18-
&Rc::new(RefCell::new(self.clone())).as_pointer(),
19-
Rc::new(RefCell::new(other.clone())).as_pointer(),
18+
&Rc::new(RefCell::new(Pair {
19+
x: self.x.clone(),
20+
y: self.y.clone(),
21+
}))
22+
.as_pointer(),
23+
Rc::new(RefCell::new(Pair {
24+
x: other.x.clone(),
25+
y: other.y.clone(),
26+
}))
27+
.as_pointer(),
2028
) {
2129
std::cmp::Ordering::Less
2230
} else if PairImpl::operator_lt(
23-
&Rc::new(RefCell::new(other.clone())).as_pointer(),
24-
Rc::new(RefCell::new(self.clone())).as_pointer(),
31+
&Rc::new(RefCell::new(Pair {
32+
x: other.x.clone(),
33+
y: other.y.clone(),
34+
}))
35+
.as_pointer(),
36+
Rc::new(RefCell::new(Pair {
37+
x: self.x.clone(),
38+
y: self.y.clone(),
39+
}))
40+
.as_pointer(),
2541
) {
2642
std::cmp::Ordering::Greater
2743
} else {
@@ -39,11 +55,27 @@ impl std::cmp::PartialEq for Pair {
3955
fn eq(&self, other: &Self) -> bool {
4056
{
4157
!(PairImpl::operator_lt(
42-
&Rc::new(RefCell::new(self.clone())).as_pointer(),
43-
Rc::new(RefCell::new(other.clone())).as_pointer(),
58+
&Rc::new(RefCell::new(Pair {
59+
x: self.x.clone(),
60+
y: self.y.clone(),
61+
}))
62+
.as_pointer(),
63+
Rc::new(RefCell::new(Pair {
64+
x: other.x.clone(),
65+
y: other.y.clone(),
66+
}))
67+
.as_pointer(),
4468
)) && !(PairImpl::operator_lt(
45-
&Rc::new(RefCell::new(other.clone())).as_pointer(),
46-
Rc::new(RefCell::new(self.clone())).as_pointer(),
69+
&Rc::new(RefCell::new(Pair {
70+
x: other.x.clone(),
71+
y: other.y.clone(),
72+
}))
73+
.as_pointer(),
74+
Rc::new(RefCell::new(Pair {
75+
x: self.x.clone(),
76+
y: self.y.clone(),
77+
}))
78+
.as_pointer(),
4779
))
4880
}
4981
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ impl std::cmp::Ord for S {
1414
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
1515
{
1616
SImpl::operator_cmp(
17-
&Rc::new(RefCell::new(self.clone())).as_pointer(),
18-
Rc::new(RefCell::new(other.clone())).as_pointer(),
17+
&Rc::new(RefCell::new(S { v: self.v.clone() })).as_pointer(),
18+
Rc::new(RefCell::new(S { v: other.v.clone() })).as_pointer(),
1919
)
2020
}
2121
}
@@ -29,8 +29,8 @@ impl std::cmp::PartialEq for S {
2929
fn eq(&self, other: &Self) -> bool {
3030
{
3131
SImpl::operator_eq(
32-
&Rc::new(RefCell::new(self.clone())).as_pointer(),
33-
Rc::new(RefCell::new(other.clone())).as_pointer(),
32+
&Rc::new(RefCell::new(S { v: self.v.clone() })).as_pointer(),
33+
Rc::new(RefCell::new(S { v: other.v.clone() })).as_pointer(),
3434
)
3535
}
3636
}

0 commit comments

Comments
 (0)