Fix double borrowing in unsafe mode

Author: nunoplopes

cpp2rust/converter/converter.cpp

@@ -3374,8 +3374,46 @@ bool Converter::ConvertCXXOperatorCallExpr(clang::CXXOperatorCallExpr *expr) {
     }
     break;
   case clang::OverloadedOperatorKind::OO_Subscript: {
+    auto *base = expr->getArg(0);
+    auto *idx = expr->getArg(1);
+    // Rust borrows the container before computing the index, so an index that
+    // reads the container, or that has side effects through anything the
+    // container is reached through, is computed beforehand.
+    std::unordered_set<const clang::ValueDecl *> base_vars;
+    auto *inner = base->IgnoreParenImpCasts();
+    if (idx->HasSideEffects(ctx_)) {
+      base_vars = GetAllVars(base);
+    } else if (auto *ref = clang::dyn_cast<clang::DeclRefExpr>(inner)) {
+      base_vars.insert(ref->getDecl());
+    } else if (auto *member = clang::dyn_cast<clang::MemberExpr>(inner)) {
+      base_vars.insert(member->getMemberDecl());
+    } else {
+      base_vars = GetAllVars(base);
+    }
+    if (!IsUniquePtr(base->getType()) &&
+        std::ranges::any_of(GetAllVars(idx), [&](const clang::ValueDecl *var) {
+          return base_vars.contains(var);
+        })) {
+      std::string idx_str;
+      {
+        PushExplicitAutoref no_autoref(*this, std::nullopt);
+        idx_str = ConvertRValue(idx);
+      }
+      PushParen paren(*this);
+      StrCat(token::kStar);
+      PushBrace brace(*this);
+      StrCat("let __idx =", idx_str);
+      if (Mapper::Map(*this, idx->getType()) != "usize") {
+        StrCat(keyword::kAs, "usize");
+      }
+      StrCat(token::kSemiColon, IsMutatingCall(expr) ? "&mut" : "&");
+      PushExplicitAutoref autoref(*this, IsMutatingCall(expr));
+      Convert(base->IgnoreImplicit());
+      StrCat("[__idx]");
+      break;
+    }
     PushExplicitAutoref autoref(*this, IsMutatingCall(expr));
-    ConvertArraySubscript(expr->getArg(0), expr->getArg(1), expr->getType());
+    ConvertArraySubscript(base, idx, expr->getType());
     break;
   }
   case clang::OverloadedOperatorKind::OO_LessLess:

tests/unit/out/unsafe/vector_subscript_borrow.rs

@@ -0,0 +1,65 @@
+extern crate libc;
+use libc::*;
+extern crate libcc2rs;
+use libcc2rs::*;
+use std::collections::BTreeMap;
+use std::io::{Read, Seek, Write};
+use std::os::fd::{AsFd, FromRawFd, IntoRawFd};
+use std::rc::Rc;
+pub static mut next_0: std::cell::LazyCell<i32> = std::cell::LazyCell::new(|| unsafe { 0 });
+pub unsafe fn bump_1(mut v: *mut Vec<i32>, mut x: i32) -> i32 {
+    (&mut (*v))[(0_usize)] += x;
+    return (*std::cell::LazyCell::force_mut(&mut *&raw mut next_0)).postfix_inc();
+}
+pub fn main() {
+    unsafe {
+        __cpp2rust_init_globals();
+        std::process::exit(main_0() as i32);
+    }
+}
+unsafe fn main_0() -> i32 {
+    let mut v: Vec<i32> = vec![1; 4_usize as usize];
+    let mut items: Vec<u32> = (0..(3_usize) as usize)
+        .map(|_| <u32>::default())
+        .collect::<Vec<_>>();
+    let mut i: u32 = 0_u32;
+    'loop_: while ((i as usize) < (items.len())) {
+        items[(i as usize)] = (i).wrapping_mul(2_u32);
+        i.prefix_inc();
+    }
+    items[(1_usize)] = (items[(1_usize)]).wrapping_add(5_u32);
+    items[(2_usize)].postfix_inc();
+    assert!(
+        (((items[(0_usize)]) == (0_u32)) && ((items[(1_usize)]) == (7_u32)))
+            && ((items[(2_usize)]) == (5_u32))
+    );
+    (*{
+        let __idx = (v[(0_usize)] as usize);
+        &mut v[__idx]
+    }) = 3;
+    (*{
+        let __idx = (v[(1_usize)] as usize);
+        &mut v[__idx]
+    }) = ((v[(1_usize)]) + (4));
+    assert!(((v[(1_usize)]) == (3)) && ((v[(3_usize)]) == (7)));
+    let mut p: *mut i32 = (&mut v[(0_usize)] as *mut i32);
+    v[((*p) as usize)] = 9;
+    assert!(((v[(1_usize)]) == (9)));
+    (*{
+        let __idx = ((unsafe { bump_1((&mut v as *mut Vec<i32>), 10) }) as usize);
+        &mut v[__idx]
+    }) = 2;
+    assert!(((v[(0_usize)]) == (2)));
+    assert!(
+        ((unsafe {
+            let _v: *mut Vec<i32> = (&mut v as *mut Vec<i32>);
+            let _x: i32 = v[(2_usize)];
+            bump_1(_v, _x)
+        }) == (1))
+            && ((v[(0_usize)]) == (3))
+    );
+    return 0;
+}
+pub unsafe fn __cpp2rust_init_globals() {
+    std::cell::LazyCell::force(&*&raw const next_0);
+}

tests/unit/out/unsafe/vector_subscript_direct.rs

@@ -7,7 +7,7 @@ use std::io::{Read, Seek, Write};
 use std::os::fd::{AsFd, FromRawFd, IntoRawFd};
 use std::rc::Rc;
 #[repr(C)]
-#[derive(Copy, Clone, Default)]
+#[derive(Copy, Clone, VaArg, FnPtrArg, Default)]
 pub struct Point {
     pub x: i32,
     pub y: i32,
@@ -18,13 +18,16 @@ impl Point {
     }
 }
 #[repr(C)]
-#[derive(Clone, Default)]
+#[derive(Clone, VaArg, FnPtrArg, Default)]
 pub struct Holder {
     pub values: Vec<i32>,
     pub points: Vec<Point>,
 }
 pub unsafe fn push_and_index_0(v: *mut Vec<i32>) -> i32 {
-    (*v).push(42);
+    {
+        let __a1 = 42;
+        (*v).push(__a1)
+    };
     return (((*v).len() as i32) - (1));
 }
 pub unsafe fn sum_ref_1(v: *const Vec<i32>) -> i32 {
@@ -52,12 +55,25 @@ unsafe fn main_0() -> i32 {
     v[(0_usize)] = v[(1_usize)];
     assert!(((v[(0_usize)]) == (7)));
     v[(1_usize)] = 0;
-    assert!(((v[(v[(1_usize)] as usize)]) == (7)));
+    assert!(
+        ((*{
+            let __idx = (v[(1_usize)] as usize);
+            &mut v[__idx]
+        }) == (7))
+    );
     let mut i: i32 = 0;
     v[(i.postfix_inc() as usize)] = 3;
     assert!(((i) == (1)) && ((v[(0_usize)]) == (3)));
-    assert!(((v[((unsafe { push_and_index_0(&mut v,) }) as usize)]) == (42)));
-    v[((unsafe { push_and_index_0(&mut v) }) as usize)] = 5;
+    assert!(
+        ((*{
+            let __idx = ((unsafe { push_and_index_0(&mut v) }) as usize);
+            &mut v[__idx]
+        }) == (42))
+    );
+    (*{
+        let __idx = ((unsafe { push_and_index_0(&mut v) }) as usize);
+        &mut v[__idx]
+    }) = 5;
     assert!(((v.len()) == (5_usize)) && ((v[(4_usize)]) == (5)));
     let mut p: *mut i32 = (&mut v[(2_usize)] as *mut i32);
     (*p) = 9;
@@ -69,7 +85,10 @@ unsafe fn main_0() -> i32 {
         h.values.resize_with(__a0, || <i32>::default())
     };
     h.values[(1_usize)] = 6;
-    h.points.push(Point { x: 1, y: 2 });
+    {
+        let __a1 = Point { x: 1, y: 2 };
+        h.points.push(__a1)
+    };
     h.points[(0_usize)].y = 5;
     assert!(((h.values[(1_usize)]) == (6)));
     assert!(((unsafe { Point::sum(&h.points[(0_usize)],) }) == (6)));
@@ -80,9 +99,20 @@ unsafe fn main_0() -> i32 {
     let mut q: Point = h.points[(0_usize)];
     q.x = 0;
     assert!(((h.points[(0_usize)].x) == (7)));
-    let mut grid: Vec<Vec<i32>> = vec![vec![0; 3_usize as usize]; 2_usize as usize];
+    let mut grid: Vec<Vec<i32>> = Vec::new();
+    {
+        let __a1 = vec![0; 3_usize as usize];
+        grid.push(__a1)
+    };
+    {
+        let __a1 = vec![0; 3_usize as usize];
+        grid.push(__a1)
+    };
     grid[(1_usize)][(2_usize)] = 8;
     assert!(((grid[(1_usize)][(2_usize)]) == (8)) && ((grid[(0_usize)][(2_usize)]) == (0)));
+    grid[(0_usize)][(1_usize)] = grid[(1_usize)][(2_usize)];
+    grid[(1_usize)][(0_usize)] += grid[(0_usize)][(1_usize)];
+    assert!(((grid[(0_usize)][(1_usize)]) == (8)) && ((grid[(1_usize)][(0_usize)]) == (8)));
     let mut a: Vec<i32> = vec![4, 5, 6];
     a[(1_usize)] = ((a[(0_usize)]) + (a[(2_usize)]));
     assert!(((a[(1_usize)]) == (10)));

tests/unit/vector_subscript_borrow.cpp

@@ -1,4 +1,3 @@
-// no-compile: unsafe
 #include <cassert>
 #include <vector>
 

tests/unit/vector_subscript_direct.cpp

@@ -1,5 +1,3 @@
-// The unsafe model cannot express v[push_and_index(v)] without two borrows.
-// no-compile: unsafe
 #include <array>
 #include <cassert>
 #include <vector>