Unbox local vectors and issue straight [] accesses instead of going through Ptr

Author: nunoplopes

cpp2rust/converter/models/boxed_vars.cpp

@@ -5,20 +5,41 @@
 
 #include <clang/AST/RecursiveASTVisitor.h>
 
+#include <algorithm>
 #include <unordered_map>
 #include <vector>
 
 #include "converter/converter_lib.h"
 #include "converter/mapper.h"
+#include "converter/rules/registry.h"
 
 namespace cpp2rust {
 namespace {
+// Whether type is translated to a Vec whose elements are not in Values:
+// std::vector and std::string of scalars or user-defined structs.
+bool IsFlatVec(clang::QualType type) {
+  auto *record =
+      clang::dyn_cast_or_null<clang::ClassTemplateSpecializationDecl>(
+          type->getAsCXXRecordDecl());
+  if (!record || !record->isInStdNamespace() ||
+      (record->getName() != "vector" && record->getName() != "basic_string")) {
+    return false;
+  }
+  auto element = record->getTemplateArgs()[0].getAsType();
+  return element->isScalarType() ||
+         (element->isStructureOrClassType() &&
+          IsUserDefinedDecl(element->getAsRecordDecl()));
+}
+
 // Whether decl may be stored without a Value, if its address is not taken:
-// scalars, arrays of non-arrays, user-defined structs and unique_ptrs, unless
-// they have a destructor, which is called with a pointer to them.
+// scalars, arrays of non-arrays, user-defined structs, unique_ptrs and
+// vectors, unless they have a destructor, which is called with a pointer to
+// them.
 bool CanUnbox(const clang::VarDecl *decl) {
   auto type = decl->getType();
-  if (type->isConstantArrayType()) {
+  if (IsFlatVec(type)) {
+    // Vectors are flat Vecs.
+  } else if (type->isConstantArrayType()) {
     if (type->getAsArrayTypeUnsafe()->getElementType()->isArrayType()) {
       return false;
     }
@@ -72,8 +93,9 @@ bool IsTrivialAssignment(clang::CXXOperatorCallExpr *expr) {
 class AddressTakenVisitor
     : public clang::RecursiveASTVisitor<AddressTakenVisitor> {
 public:
-  explicit AddressTakenVisitor(std::unordered_set<const clang::VarDecl *> &vars)
-      : address_taken_(vars) {}
+  AddressTakenVisitor(clang::ASTContext &ctx,
+                      std::unordered_set<const clang::VarDecl *> &vars)
+      : ctx_(ctx), address_taken_(vars) {}
 
   bool shouldVisitTemplateInstantiations() const { return true; }
 
@@ -110,12 +132,18 @@ class AddressTakenVisitor
     if (auto *source = GetTrivialCopySource(expr)) {
       AddValueUse(source);
     }
+    AddRuleArgs(expr, expr->getArgs(), expr->getNumArgs());
+    return true;
+  }
+
+  bool VisitCallExpr(clang::CallExpr *expr) {
+    AddRuleArgs(expr, expr->getArgs(), expr->getNumArgs());
     return true;
   }
 
   bool VisitCXXOperatorCallExpr(clang::CXXOperatorCallExpr *expr) {
     if (IsTrivialAssignment(expr)) {
-      AddValueUse(expr->getArg(0));
+      AddValueUse(expr->getArg(0), /*write=*/true);
       AddValueUse(IgnoreNoOpCasts(expr->getArg(1)));
     }
     // Dereferencing a unique_ptr reads the pointer it holds; the object it
@@ -136,14 +164,14 @@ class AddressTakenVisitor
 
   bool VisitBinaryOperator(clang::BinaryOperator *expr) {
     if (expr->isAssignmentOp()) {
-      AddValueUse(expr->getLHS());
+      AddValueUse(expr->getLHS(), /*write=*/true);
     }
     return true;
   }
 
   bool VisitUnaryOperator(clang::UnaryOperator *expr) {
     if (expr->isIncrementDecrementOp()) {
-      AddValueUse(expr->getSubExpr());
+      AddValueUse(expr->getSubExpr(), /*write=*/true);
     }
     return true;
   }
@@ -165,21 +193,48 @@ class AddressTakenVisitor
   }
 
 private:
-  // An element of an array, or a field of a struct, is accessed without
-  // making a pointer to it.
-  void AddValueUse(clang::Expr *expr) {
+  // The arguments of a call translated by a rule are passed by value or
+  // borrowed, unless the rule takes a pointer to them.
+  void AddRuleArgs(clang::Expr *expr, clang::Expr **args, unsigned num_args) {
+    auto *rule = RuleRegistry::GetExprRule(ctx_, GetCalleeOrExpr(expr));
+    if (!rule) {
+      return;
+    }
+    // Variadic arguments are not considered.
+    auto all_args = BuildUnifiedArgs(expr, args, num_args);
+    for (unsigned i = 0; i < all_args.size() && i < rule->params.size(); ++i) {
+      if (!rule->params[i].is_pointer()) {
+        AddValueUse(IgnoreNoOpCasts(all_args[i]));
+      }
+    }
+  }
+
+  // An element of an array or vector, or a field of a struct, is accessed
+  // without making a pointer to it.
+  void AddValueUse(clang::Expr *expr, bool write = false) {
     expr = expr->IgnoreParens();
     if (auto *member = clang::dyn_cast<clang::MemberExpr>(expr)) {
       if (!member->isArrow() &&
           clang::isa<clang::FieldDecl>(member->getMemberDecl())) {
-        AddValueUse(member->getBase());
+        AddValueUse(member->getBase(), write);
       }
     } else if (auto *subscript =
                    clang::dyn_cast<clang::ArraySubscriptExpr>(expr)) {
       auto *cast =
           clang::dyn_cast<clang::ImplicitCastExpr>(subscript->getBase());
       if (cast && cast->getCastKind() == clang::CK_ArrayToPointerDecay) {
-        AddValueUse(cast->getSubExpr());
+        AddValueUse(cast->getSubExpr(), write);
+      }
+    } else if (auto *op = clang::dyn_cast<clang::CXXOperatorCallExpr>(expr)) {
+      // Rust doesn't let the index of an element that is written to read the
+      // vector, as the vector is borrowed mutably first.
+      auto *vec = IgnoreNoOpCasts(op->getArg(0));
+      if (op->getOperator() == clang::OO_Subscript &&
+          IsFlatVec(vec->getType()) &&
+          (!write || std::ranges::none_of(GetAllVars(vec), [&](auto *var) {
+            return GetAllVars(op->getArg(1)).contains(var);
+          }))) {
+        AddValueUse(vec, write);
       }
     } else if (auto *ref = clang::dyn_cast<clang::DeclRefExpr>(expr)) {
       value_uses_.insert(ref);
@@ -193,14 +248,15 @@ class AddressTakenVisitor
     }
   }
 
+  clang::ASTContext &ctx_;
   std::unordered_set<const clang::VarDecl *> &address_taken_;
   // The references to variables that only access their value.
   std::unordered_set<const clang::DeclRefExpr *> value_uses_;
 };
 } // namespace
 
 BoxedVars::BoxedVars(clang::ASTContext &ctx) {
-  AddressTakenVisitor(address_taken_)
+  AddressTakenVisitor(ctx, address_taken_)
       .TraverseDecl(ctx.getTranslationUnitDecl());
 }
 

cpp2rust/converter/models/converter_refcount.cpp

@@ -208,6 +208,14 @@ bool ConverterRefCount::IsUnboxedPlace(const clang::Expr *expr) const {
   return ref && IsUnboxedVar(ref->getDecl());
 }
 
+bool ConverterRefCount::IsValueLocal(const clang::Expr *expr) const {
+  auto *ref = clang::dyn_cast<clang::DeclRefExpr>(expr->IgnoreParenImpCasts());
+  auto *var = ref ? clang::dyn_cast<clang::VarDecl>(ref->getDecl()) : nullptr;
+  // Variables captured by reference are accessed through pointers.
+  return var && !IsGlobalVar(var) && !IsUnboxedVar(var) &&
+         !GetDeclRefType(curr_function_, ref, var)->isReferenceType();
+}
+
 bool ConverterRefCount::IsFieldOfUnboxedPlace(
     const clang::MemberExpr *expr) const {
   return !expr->isArrow() && !expr->getBase()->getType()->isUnionType() &&
@@ -2766,6 +2774,37 @@ bool ConverterRefCount::ConvertCXXOperatorCallExpr(
         IsBoxedType(ctx_, expr->getType().getNonReferenceType()) &&
         IsBoxedType(ctx_, expr->getArg(0)->getType().getNonReferenceType());
 
+    // An element of an unboxed container is accessed directly, as is one of
+    // a container stored in a Value by borrowing the container, unless
+    // computing the index may access the container too, as it stays borrowed
+    // meanwhile.
+    auto *idx = expr->getArg(1);
+    bool is_unboxed =
+        IsUnboxedPlace(expr->getArg(0)) &&
+        clang::isa<clang::DeclRefExpr>(expr->getArg(0)->IgnoreParenImpCasts());
+    if ((isLValue() || isRValue()) && !is_inner_boxed &&
+        (is_unboxed ||
+         (IsValueLocal(expr->getArg(0)) && !idx->HasSideEffects(ctx_) &&
+          std::ranges::none_of(
+              GetAllVars(idx), [&](const clang::ValueDecl *var) {
+                return var->getType()->isPointerType() ||
+                       var->getType()->isReferenceType() ||
+                       GetAllVars(expr->getArg(0)).contains(var);
+              })))) {
+      // The index is a size_t, i.e., a usize.
+      auto str =
+          std::format("{}[{}]", ToString(expr->getArg(0)), ConvertRValue(idx));
+      // A copied element is read in a block, which ends the borrow.
+      if (!is_unboxed && isRValue() && TypeIsCopyable(expr->getType())) {
+        StrCat(std::format("{{ {} }}", str));
+        SetFreshType(expr->getType());
+      } else {
+        StrCat(str);
+        SetValueFreshness(expr->getType());
+      }
+      break;
+    }
+
     if (isLValue()) {
       PushConversionKind push_ck(*this, ConversionKind::Unboxed);
       pending_deref_.set(

cpp2rust/converter/models/converter_refcount.h

@@ -425,6 +425,8 @@ class ConverterRefCount final : public Converter {
   // Whether expr is an unboxed variable or a field of one, which is accessed
   // in place.
   bool IsUnboxedPlace(const clang::Expr *expr) const;
+  // Whether expr is a local variable or parameter stored in a Value.
+  bool IsValueLocal(const clang::Expr *expr) const;
   // Whether expr is a field of an unboxed variable, or of a field of one,
   // which is stored in place, even if it is a reference.
   bool IsFieldOfUnboxedPlace(const clang::MemberExpr *expr) const;

tests/multi-file/defaulted_move_cross_tu/out/refcount/defaulted_move_cross_tu.rs

@@ -17,14 +17,10 @@ pub struct S {
     pub n: Value<Box<[i32]>>,
 }
 impl S {
-    pub fn new(x: i32) -> Self {
-        let x: Value<i32> = Rc::new(RefCell::new(x));
+    pub fn new(mut x: i32) -> Self {
         Self {
-            v: Rc::new(RefCell::new(vec![
-                (*x.borrow());
-                ((*x.borrow()) as usize) as usize
-            ])),
-            n: Rc::new(RefCell::new(Box::new([(*x.borrow()), ((*x.borrow()) + 1)]))),
+            v: Rc::new(RefCell::new(vec![x; (x as usize) as usize])),
+            n: Rc::new(RefCell::new(Box::new([x, (x + 1)]))),
         }
     }
     pub fn move_from(_a0: Ptr<S>) -> Self {

tests/unit/out/refcount/07_unique.rs

@@ -6,10 +6,9 @@ use std::io::prelude::*;
 use std::io::{Read, Seek, Write};
 use std::os::fd::AsFd;
 use std::rc::{Rc, Weak};
-pub fn fn_0(u: Option<Value<i32>>) -> Option<Value<i32>> {
-    let u: Value<Option<Value<i32>>> = Rc::new(RefCell::new(u));
-    (*(*u.borrow()).as_ref().unwrap().borrow_mut()) = 10;
-    return (*u.borrow_mut()).take();
+pub fn fn_0(mut u: Option<Value<i32>>) -> Option<Value<i32>> {
+    (*u.as_ref().unwrap().borrow_mut()) = 10;
+    return u.take();
 }
 pub fn main() {
     __cpp2rust_init_globals();

tests/unit/out/refcount/08_unique_array.rs

@@ -11,17 +11,17 @@ pub fn main() {
     std::process::exit(main_0());
 }
 fn main_0() -> i32 {
-    let g: Value<Option<Value<Box<[i32]>>>> = Rc::new(RefCell::new(Some(Rc::new(RefCell::new(
+    let mut g: Option<Value<Box<[i32]>>> = Some(Rc::new(RefCell::new(
         (0..2_usize).map(|_| <i32>::default()).collect::<Box<[_]>>(),
-    )))));
-    (*g.borrow()).as_ref().unwrap().borrow_mut()[(0_usize) as usize] = 11;
-    (*g.borrow()).as_ref().unwrap().borrow_mut()[(1_usize) as usize] = 12;
-    let mut g_ptr: Ptr<i32> = (*g.borrow()).as_pointer();
+    )));
+    g.as_ref().unwrap().borrow_mut()[(0_usize) as usize] = 11;
+    g.as_ref().unwrap().borrow_mut()[(1_usize) as usize] = 12;
+    let mut g_ptr: Ptr<i32> = g.as_pointer();
     elem!(g_ptr, 0).write(13);
     elem!(g_ptr, 1).write(14);
     assert!(
-        (((*g.borrow()).as_ref().unwrap().borrow()[(0_usize) as usize]
-            + (*g.borrow()).as_ref().unwrap().borrow()[(1_usize) as usize])
+        ((g.as_ref().unwrap().borrow()[(0_usize) as usize]
+            + g.as_ref().unwrap().borrow()[(1_usize) as usize])
             == 27)
     );
     return 0;

tests/unit/out/refcount/array_of_noncopy_struct.rs

@@ -28,28 +28,24 @@ pub fn main() {
     std::process::exit(main_0());
 }
 fn main_0() -> i32 {
-    let arr: Value<Box<[NonCopy]>> = Rc::new(RefCell::new(
-        (0..3)
-            .map(|_| <NonCopy>::default())
-            .collect::<Box<[NonCopy]>>(),
-    ));
-    (*arr.borrow_mut())[(0) as usize].tag = 7;
+    let mut arr: [NonCopy; 3] = std::array::from_fn::<_, 3, _>(|_| <NonCopy>::default());
+    arr[(0) as usize].tag = 7;
     {
         let __a1 = 42;
-        (*{ (*arr.borrow())[(1) as usize].data.clone() }.borrow_mut()).push(__a1)
+        (*{ arr[(1) as usize].data.clone() }.borrow_mut()).push(__a1)
     };
-    assert!(({ (*arr.borrow())[(0) as usize].tag } == 7));
-    assert!(((*{ (*arr.borrow())[(1) as usize].data.clone() }.borrow()).len() == 1_usize));
+    assert!(({ arr[(0) as usize].tag } == 7));
+    assert!(((*{ arr[(1) as usize].data.clone() }.borrow()).len() == 1_usize));
     assert!(
         ((elem!(
-            ({ (*arr.borrow())[(1) as usize].data.as_pointer() } as Ptr<i32>),
+            ({ arr[(1) as usize].data.as_pointer() } as Ptr<i32>),
             0_usize
         )
         .read())
             == 42)
     );
-    assert!(({ (*arr.borrow())[(2) as usize].tag } == 0));
-    assert!(((*{ (*arr.borrow())[(2) as usize].data.clone() }.borrow()).len() == 0_usize));
+    assert!(({ arr[(2) as usize].tag } == 0));
+    assert!(((*{ arr[(2) as usize].data.clone() }.borrow()).len() == 0_usize));
     return 0;
 }
 pub fn __cpp2rust_init_globals() {}

tests/unit/out/refcount/byte.rs

@@ -11,27 +11,27 @@ pub fn main() {
     std::process::exit(main_0());
 }
 fn main_0() -> i32 {
-    let b1: Value<u8> = Rc::new(RefCell::new(1_u8));
+    let mut b1: u8 = 1_u8;
     let mut ushift1: u32 = 3_u32;
-    let mut shl1: u8 = (*b1.borrow()) << ushift1;
+    let mut shl1: u8 = b1 << ushift1;
     assert!((shl1 == ((8) as u8)));
     let mut ushift2: u32 = 2_u32;
     let mut shr1: u8 = shl1 >> ushift2;
     assert!((shr1 == ((2) as u8)));
     let mut ushift3: u32 = 5_u32;
     {
-        let n_ = (*b1.borrow()) << ushift3;
-        (*b1.borrow_mut()) = n_;
-        (*b1.borrow())
+        let n_ = b1 << ushift3;
+        b1 = n_;
+        b1
     };
-    assert!(((*b1.borrow()) == ((32) as u8)));
+    assert!((b1 == ((32) as u8)));
     let mut ushift4: u32 = 3_u32;
     {
-        let n_ = (*b1.borrow()) >> ushift4;
-        (*b1.borrow_mut()) = n_;
-        (*b1.borrow())
+        let n_ = b1 >> ushift4;
+        b1 = n_;
+        b1
     };
-    assert!(((*b1.borrow()) == ((4) as u8)));
+    assert!((b1 == ((4) as u8)));
     return 0;
 }
 pub fn __cpp2rust_init_globals() {}

tests/unit/out/refcount/c_string_embedded_nul.rs

@@ -40,32 +40,31 @@ fn main_0() -> i32 {
             false => -1,
         }
     };
-    let s: Value<Vec<i8>> = Rc::new(RefCell::new({
+    let mut s: Vec<i8> = {
         let mut __bytes = (buf.as_pointer() as Ptr<i8>).to_c_bytes();
         __bytes.push(0);
         __bytes
-    }));
-    println!("{}", ((*s.borrow()).len() - 1));
+    };
+    println!("{}", (s.len() - 1));
     println!(
         "{} {}",
         (buf.as_pointer() as Ptr::<i8>)
             .to_c_string_iterator()
             .count(),
         (buf.as_pointer() as Ptr::<i8>).with_c_str(|__lookup| {
-            (*s.borrow())
-                .iter()
-                .take((*s.borrow()).len().saturating_sub(1))
+            s.iter()
+                .take(s.len().saturating_sub(1))
                 .rposition(|&x| __lookup.contains(&x))
                 .unwrap_or(usize::MAX)
         })
     );
     let mut lit: Ptr<i8> = Ptr::<i8>::from_string_literal(b"xy\0zw");
-    let t: Value<Vec<i8>> = Rc::new(RefCell::new({
+    let mut t: Vec<i8> = {
         let mut __bytes = lit.to_c_bytes();
         __bytes.push(0);
         __bytes
-    }));
-    println!("{}", ((*t.borrow()).len() - 1));
+    };
+    println!("{}", (t.len() - 1));
     return 0;
 }
 pub fn __cpp2rust_init_globals() {}