Skip to content

Commit 22edd84

Browse files
authored
Fix compound assignment type casts (#228)
Fixes type casts inside compound assignments under the `unsafe` model and removes unnecessary casts under the `refcount` model. When translating binary operators, both `unsafe` and `refcount` have a special case for translating compound assignments involving different types. The types being compared were the original C++ types. However, differences between the original C++ types do not necessarily mean that the corresponding Rust types differ. This caused unnecessary casts to be introduced and, under the `unsafe` model, resulted in translations that do not compile, such as the following: ```cpp int main() { std::vector<int> v; v.push_back(10); v.front() += 5; assert(v.front() == 15); return v.front(); } ``` ```rust pub fn main() { unsafe { std::process::exit(main_0() as i32); } } unsafe fn main_0() -> i32 { let mut v: Vec<i32> = Vec::new(); v.push(10); ((v).first_mut().unwrap()) = ((((v).first_mut().unwrap()) as i32) + 5) as i32; // invalid cast assert!(((*((v).first_mut().unwrap())) == (15))); return (*((v).first_mut().unwrap())); } ```
1 parent 3675d3f commit 22edd84

12 files changed

Lines changed: 86 additions & 20 deletions

cpp2rust/converter/converter.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2274,7 +2274,8 @@ void Converter::ConvertBinaryOperator(clang::BinaryOperator *expr) {
22742274
if (auto *cmpd_assign_op =
22752275
llvm::dyn_cast<clang::CompoundAssignOperator>(expr);
22762276
expr->isCompoundAssignmentOp() &&
2277-
lhs_type != cmpd_assign_op->getComputationResultType()) {
2277+
GetUnsafeTypeAsString(lhs_type) !=
2278+
GetUnsafeTypeAsString(cmpd_assign_op->getComputationResultType())) {
22782279
auto computation_result_type = cmpd_assign_op->getComputationResultType();
22792280
if (IsUnsignedArithOp(cmpd_assign_op)) {
22802281
Convert(lhs);

cpp2rust/converter/models/converter_refcount.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,14 @@ ConverterRefCount::PushUnboxedIfSimple::PushUnboxedIfSimple(
109109
: ConversionKind::FullRefCount);
110110
}
111111

112+
std::string
113+
ConverterRefCount::GetSafeTypeAsString(clang::QualType qual_type) const {
114+
std::string type_as_string;
115+
ConverterRefCount converter(type_as_string, ctx_);
116+
converter.Convert(qual_type);
117+
return std::string(Trim(type_as_string));
118+
}
119+
112120
std::string ConverterRefCount::BoxType(std::string &&str) const {
113121
switch (getConversionKind()) {
114122
case ConversionKind::Unboxed:
@@ -1362,7 +1370,8 @@ void ConverterRefCount::ConvertBinaryOperator(clang::BinaryOperator *expr) {
13621370
std::string_view opcode_as_string = expr->getOpcodeStr();
13631371

13641372
if (auto *assign = llvm::dyn_cast<clang::CompoundAssignOperator>(expr);
1365-
assign && lhs_type != assign->getComputationResultType()) {
1373+
assign && GetSafeTypeAsString(lhs_type) !=
1374+
GetSafeTypeAsString(assign->getComputationResultType())) {
13661375
auto computation_result_type = assign->getComputationResultType();
13671376
StrCat(keyword::kLet, "rhs_0", token::kAssign);
13681377
if (IsUnsignedArithOp(assign)) {

cpp2rust/converter/models/converter_refcount.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,8 @@ class ConverterRefCount final : public Converter {
242242
std::string ConvertPtrType(clang::QualType type);
243243
std::string ConvertPointeeType(clang::QualType ptr_type) override;
244244

245+
std::string GetSafeTypeAsString(clang::QualType qual_type) const;
246+
245247
/// The kind of conversion that should be performed.
246248
enum class ConversionKind : uint8_t {
247249
Unboxed,

tests/unit/compound_assign_ref.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright (c) 2022-present INESC-ID.
2+
// Distributed under the MIT license that can be found in the LICENSE file.
3+
4+
#include <cassert>
5+
#include <vector>
6+
7+
int main() {
8+
std::vector<int> v;
9+
v.push_back(10);
10+
v.front() += 5;
11+
assert(v.front() == 15);
12+
return v.front();
13+
}

tests/unit/out/refcount/auto.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ fn main_0() -> i32 {
1919
let sum: Value<i32> = Rc::new(RefCell::new(0));
2020
'loop_: for mut elem in v.as_pointer() as Ptr<i32> {
2121
let elem: Value<i32> = Rc::new(RefCell::new(elem.read().clone()));
22-
let rhs_0 = (((*sum.borrow()) as i32) + (*elem.borrow())) as i32;
23-
(*sum.borrow_mut()) = rhs_0;
22+
(*sum.borrow_mut()) += (*elem.borrow());
2423
}
2524
return (*sum.borrow());
2625
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
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 main() {
10+
std::process::exit(main_0());
11+
}
12+
fn main_0() -> i32 {
13+
let v: Value<Vec<i32>> = Rc::new(RefCell::new(Vec::new()));
14+
(*v.borrow_mut()).push(10);
15+
{
16+
let _ptr = (v.as_pointer() as Ptr<i32>).clone();
17+
_ptr.write(_ptr.read() + 5)
18+
};
19+
assert!((((v.as_pointer() as Ptr<i32>).read()) == 15));
20+
return ((v.as_pointer() as Ptr<i32>).read());
21+
}

tests/unit/out/refcount/foreach_mut.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@ fn main_0() -> i32 {
2424
(*sum.borrow_mut()) += (*x.borrow());
2525
}
2626
'loop_: for mut x in v1.as_pointer() as Ptr<i32> {
27-
let rhs_0 = (((x.read()) as i32) + 10) as i32;
28-
x.write(rhs_0);
27+
{
28+
let _ptr = x.clone();
29+
_ptr.write(_ptr.read() + 10)
30+
};
2931
}
3032
'loop_: for mut x in v1.as_pointer() as Ptr<i32> {
3133
let __rhs = (x.read());
@@ -49,8 +51,10 @@ fn main_0() -> i32 {
4951
}
5052
'loop_: for mut p in v2.as_pointer() as Ptr<Ptr<i32>> {
5153
let p: Value<Ptr<i32>> = Rc::new(RefCell::new(p.read().clone()));
52-
let rhs_0 = ((((*p.borrow()).read()) as i32) + 5) as i32;
53-
(*p.borrow()).write(rhs_0);
54+
{
55+
let _ptr = (*p.borrow()).clone();
56+
_ptr.write(_ptr.read() + 5)
57+
};
5458
}
5559
'loop_: for mut p in v2.as_pointer() as Ptr<Ptr<i32>> {
5660
let p: Value<Ptr<i32>> = Rc::new(RefCell::new(p.read().clone()));

tests/unit/out/refcount/unique_ptr_deref_ops.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,9 @@ pub fn main() {
1111
}
1212
fn main_0() -> i32 {
1313
let p: Value<Option<Value<i32>>> = Rc::new(RefCell::new(Some(Rc::new(RefCell::new(10)))));
14-
let rhs_0 = (((*(*p.borrow()).as_ref().unwrap().borrow()) as i32) + 5) as i32;
15-
(*(*p.borrow_mut()).as_ref().unwrap().borrow_mut()) = rhs_0;
16-
let rhs_0 = (((*(*p.borrow()).as_ref().unwrap().borrow()) as i32) - 3) as i32;
17-
(*(*p.borrow_mut()).as_ref().unwrap().borrow_mut()) = rhs_0;
18-
let rhs_0 = (((*(*p.borrow()).as_ref().unwrap().borrow()) as i32) * 2) as i32;
19-
(*(*p.borrow_mut()).as_ref().unwrap().borrow_mut()) = rhs_0;
14+
(*(*p.borrow_mut()).as_ref().unwrap().borrow_mut()) += 5;
15+
(*(*p.borrow_mut()).as_ref().unwrap().borrow_mut()) -= 3;
16+
(*(*p.borrow_mut()).as_ref().unwrap().borrow_mut()) *= 2;
2017
let q: Value<Option<Value<i32>>> = Rc::new(RefCell::new(Some(Rc::new(RefCell::new(1)))));
2118
let sum: Value<i32> = Rc::new(RefCell::new(
2219
((*(*p.borrow()).as_ref().unwrap().borrow()) + (*(*q.borrow()).as_ref().unwrap().borrow())),

tests/unit/out/unsafe/auto.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ unsafe fn main_0() -> i32 {
2121
let mut sum: i32 = 0;
2222
'loop_: for elem in 0..(v.len()) {
2323
let mut elem = v[elem].clone();
24-
sum = ((sum as i32) + elem) as i32;
24+
sum += elem;
2525
}
2626
return sum;
2727
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
extern crate libc;
2+
use libc::*;
3+
extern crate libcc2rs;
4+
use libcc2rs::*;
5+
use std::collections::BTreeMap;
6+
use std::io::{Read, Seek, Write};
7+
use std::os::fd::{AsFd, FromRawFd, IntoRawFd};
8+
use std::rc::Rc;
9+
pub fn main() {
10+
unsafe {
11+
std::process::exit(main_0() as i32);
12+
}
13+
}
14+
unsafe fn main_0() -> i32 {
15+
let mut v: Vec<i32> = Vec::new();
16+
v.push(10);
17+
(*((v).first_mut().unwrap())) += 5;
18+
assert!(((*((v).first_mut().unwrap())) == (15)));
19+
return (*((v).first_mut().unwrap()));
20+
}

0 commit comments

Comments
 (0)