Add offset!(ptr, idx) macro to create temporary pointers without ref counting overhead

Author: nunoplopes

cpp2rust/converter/converter_lib.cpp

@@ -1338,6 +1338,16 @@ GetAllVars(const clang::Stmt *stmt) {
   return vars;
 }
 
+bool ReadsMemory(const clang::Stmt *stmt) {
+  if (auto *cast = clang::dyn_cast<clang::ImplicitCastExpr>(stmt);
+      cast && cast->getCastKind() == clang::CK_LValueToRValue) {
+    return true;
+  }
+  return std::ranges::any_of(stmt->children(), [](const clang::Stmt *child) {
+    return child && ReadsMemory(child);
+  });
+}
+
 bool ReferencesThis(const clang::Stmt *stmt) {
   if (!stmt) {
     return false;

cpp2rust/converter/converter_lib.h

@@ -242,6 +242,9 @@ GetAllVars(const clang::Stmt *stmt);
 
 bool ReferencesThis(const clang::Stmt *stmt);
 
+// Whether evaluating `stmt` reads a value from memory, e.g., a pointer.
+bool ReadsMemory(const clang::Stmt *stmt);
+
 bool MayCauseBorrowMutError(const clang::Expr *lhs, const clang::Expr *rhs);
 
 bool ArgsMayAlias(const clang::Expr *a, const clang::Expr *b);

cpp2rust/converter/models/converter_refcount.cpp

@@ -1822,12 +1822,16 @@ bool ConverterRefCount::VisitMemberExpr(clang::MemberExpr *expr) {
     return false;
   }
 
+  // A pointer to a Value field is made without copying the Value out.
+  bool value_field_ptr = isAddrOf() && !known &&
+                         clang::isa<clang::FieldDecl>(member) &&
+                         !member->getType()->isReferenceType();
   std::string str;
   if (known) {
     str = GetMappedAsString(expr);
   } else if (clang::isa<clang::FieldDecl>(member)) {
     // The Value or the pointer that the field holds.
-    str = ReadField(expr);
+    str = ReadField(expr, value_field_ptr ? ".as_pointer()" : ".clone()");
   } else {
     Buffer buf(*this);
     PushExprKind push(*this, ExprKind::RValue);
@@ -1840,7 +1844,7 @@ bool ConverterRefCount::VisitMemberExpr(clang::MemberExpr *expr) {
     if (member->getType()->isReferenceType()) {
       computed_expr_type_ = ComputedExprType::Pointer;
     } else {
-      StrCat(".as_pointer()");
+      StrCat(value_field_ptr ? "" : ".as_pointer()");
       computed_expr_type_ = ComputedExprType::FreshPointer;
     }
     return false;
@@ -1913,7 +1917,8 @@ std::string ConverterRefCount::ConvertRecordPtr(clang::MemberExpr *expr) {
   return std::move(buf).str();
 }
 
-std::string ConverterRefCount::ReadField(clang::MemberExpr *expr) {
+std::string ConverterRefCount::ReadField(clang::MemberExpr *expr,
+                                         std::string_view copy) {
   // Unless the struct is reached through a pointer, in which case the field
   // is read in a closure, the borrow of the struct ends at the end of the
   // block.
@@ -1928,7 +1933,7 @@ std::string ConverterRefCount::ReadField(clang::MemberExpr *expr) {
     str = std::move(buf).str();
   }
   if (!TypeIsCopyable(expr->getMemberDecl()->getType())) {
-    str += ".clone()";
+    str += copy;
   }
   return ptr.empty() ? std::format("{{ {} }}", str)
                      : std::format("{}.with(|__s| {})", ptr, str);
@@ -2444,7 +2449,10 @@ void ConverterRefCount::ConvertAssignment(clang::Expr *lhs, clang::Expr *rhs,
 
   PushBrace brace(*this, isRValue());
 
-  if (MayCauseBorrowMutError(lhs, rhs)) {
+  // C++ evaluates the assigned value first, which may modify what the place
+  // is computed from, like a pointer it is reached through.
+  if (MayCauseBorrowMutError(lhs, rhs) ||
+      (rhs->HasSideEffects(ctx_) && ReadsMemory(lhs))) {
     if (CanBraceAssignedValue(lhs, rhs, assign_operator)) {
       rhs_as_string = std::format("{{ {} }}", rhs_as_string);
     } else {
@@ -2607,7 +2615,7 @@ bool ConverterRefCount::ConvertCXXOperatorCallExpr(
     if (isLValue()) {
       PushConversionKind push_ck(*this, ConversionKind::Unboxed);
       pending_deref_.set(
-          std::format("({} as {}).offset({})",
+          std::format("elem!(({} as {}), {})",
                       ConvertObject(expr->getArg(0), ObjectShape::Element),
                       ConvertPtrType(expr->getArg(0)->getType()),
                       ConvertSubscriptIndex(expr->getArg(1))),
@@ -2627,10 +2635,14 @@ bool ConverterRefCount::ConvertCXXOperatorCallExpr(
       }
 
       PushConversionKind push(*this, ConversionKind::Unboxed);
-      StrCat(std::format("({} as {}).offset({})",
-                         ConvertObject(expr->getArg(0), ObjectShape::Element),
-                         ConvertPtrType(expr->getArg(0)->getType()),
-                         ConvertSubscriptIndex(expr->getArg(1))));
+      auto ptr =
+          std::format("({} as {})",
+                      ConvertObject(expr->getArg(0), ObjectShape::Element),
+                      ConvertPtrType(expr->getArg(0)->getType()));
+      auto idx = ConvertSubscriptIndex(expr->getArg(1));
+      StrCat(deref && !is_inner_boxed
+                 ? std::format("elem!({}, {})", ptr, idx)
+                 : std::format("{}.offset({})", ptr, idx));
 
       if (is_inner_boxed) {
         StrCat(GetPointerDerefSuffix(expr->getType()), ".as_pointer()");
@@ -2757,6 +2769,12 @@ void ConverterRefCount::ConvertArraySubscript(clang::Expr *base,
   }
 }
 
+void ConverterRefCount::ConvertPointerElem(clang::Expr *base,
+                                           clang::Expr *idx) {
+  StrCat(std::format("elem!({}, {})", ToString(base), ConvertRValue(idx)));
+  computed_expr_type_ = ComputedExprType::FreshPointer;
+}
+
 void ConverterRefCount::ConvertPointerSubscript(
     clang::ArraySubscriptExpr *expr) {
   auto *base = expr->getBase();
@@ -2765,7 +2783,7 @@ void ConverterRefCount::ConvertPointerSubscript(
   if (isLValue()) {
     pending_deref_.assert_consumed();
     Buffer buf(*this);
-    ConvertPointerOffset(base, idx);
+    ConvertPointerElem(base, idx);
     pending_deref_.set_unchecked(std::move(buf).str(), isFresh(), expr);
     return;
   }
@@ -2774,8 +2792,10 @@ void ConverterRefCount::ConvertPointerSubscript(
   PushParen paren(*this, deref);
   if (deref) {
     StrCat(GetPointerDerefPrefix(expr->getType()));
+    ConvertPointerElem(base, idx);
+  } else {
+    ConvertPointerOffset(base, idx);
   }
-  ConvertPointerOffset(base, idx);
   if (deref) {
     StrCat(GetPointerDerefSuffix(expr->getType()));
     SetValueFreshness(expr->getType());

cpp2rust/converter/models/converter_refcount.h

@@ -155,7 +155,8 @@ class ConverterRefCount final : public Converter {
 
   // Copies the value of the field `expr` out of its struct, such that the
   // struct doesn't stay borrowed.
-  std::string ReadField(clang::MemberExpr *expr);
+  std::string ReadField(clang::MemberExpr *expr,
+                        std::string_view copy = ".clone()");
 
   // A field that is read through a pointer to its struct is read in a
   // closure, `p.with(|__s| __s.x)`. While converting the struct whose field
@@ -387,6 +388,9 @@ class ConverterRefCount final : public Converter {
 
   std::string ConvertSubscriptIndex(clang::Expr *idx);
 
+  // Element `idx` of pointer `base`, accessed without creating a pointer to it.
+  void ConvertPointerElem(clang::Expr *base, clang::Expr *idx);
+
   std::string GetSafeTypeAsString(clang::QualType qual_type) const;
 
   bool NeedsMut(const clang::VarDecl *decl, clang::QualType type,

libcc2rs/src/field.rs

@@ -17,7 +17,7 @@ use std::any::{Any, TypeId};
 use std::cell::{Cell, Ref, RefCell, RefMut};
 use std::rc::{Rc, Weak};
 
-use crate::rc::{AsPointer, Ptr, PtrKind, Value, null_deref};
+use crate::rc::{AsPointer, Ptr, PtrKind, StrongPtr, Value, null_deref};
 use crate::reinterpret::ByteRepr;
 
 // A struct whose fields can be pointed to. Implemented by #[derive(Record)],
@@ -326,6 +326,79 @@ macro_rules! field {
     };
 }
 
+// Element `idx` of the sequence that pointer `base` points into, which is
+// accessed through `base` without creating a pointer to it as
+// `base.offset(idx)` does. Written by `elem!(p, i)`.
+pub struct ElemPlace<'a, T> {
+    pub base: &'a Ptr<T>,
+    pub idx: isize,
+}
+
+impl<T> Clone for ElemPlace<'_, T> {
+    #[inline]
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<T> Copy for ElemPlace<'_, T> {}
+
+impl<T: ByteRepr> ElemPlace<'_, T> {
+    #[inline]
+    pub fn with<R>(&self, f: impl FnOnce(&T) -> R) -> R {
+        self.base.with_at(self.idx, f)
+    }
+
+    #[inline]
+    pub fn with_mut<R>(&self, f: impl FnOnce(&mut T) -> R) -> R {
+        self.base.with_mut_at(self.idx, f)
+    }
+
+    #[inline]
+    pub fn read(&self) -> T
+    where
+        T: Clone,
+    {
+        self.base.read_at(self.idx)
+    }
+
+    #[inline]
+    pub fn write(&self, value: T) {
+        self.base.write_at(self.idx, value)
+    }
+
+    #[inline]
+    pub fn upgrade(&self) -> StrongPtr<T> {
+        self.base.offset(self.idx).upgrade()
+    }
+}
+
+impl<T: ByteRepr> Place for ElemPlace<'_, T> {
+    type Target = T;
+    #[inline]
+    fn with<R>(&self, f: impl FnOnce(&T) -> R) -> R {
+        ElemPlace::with(self, f)
+    }
+    #[inline]
+    fn with_mut<R>(&self, f: impl FnOnce(&mut T) -> R) -> R {
+        ElemPlace::with_mut(self, f)
+    }
+}
+
+// Element `idx` of the sequence that pointer `base` points into, e.g.,
+// `elem!(p, i).write(1)` for `p[i] = 1`. It is a struct expression, like
+// field!, so that a temporary base lives as long as the element when bound to
+// a variable.
+#[macro_export]
+macro_rules! elem {
+    ($base:expr, $idx:expr) => {
+        $crate::ElemPlace {
+            base: &$base,
+            idx: ($idx) as isize,
+        }
+    };
+}
+
 // Finds the object at a byte offset of a field for Record::locate. The
 // implementation is chosen based on the type of the field, by autoref-based
 // specialization: `(&&Locate(&field)).locate(...)` picks LocateRecord if the

libcc2rs/src/lib.rs

@@ -10,7 +10,7 @@ mod rc;
 pub use rc::*;
 
 mod field;
-pub use field::{FieldPlace, FieldPtr, Place, Record};
+pub use field::{ElemPlace, FieldPlace, FieldPtr, Place, Record};
 // Used by #[derive(Record)] and size_of_field!.
 #[doc(hidden)]
 pub mod __field {