Avoid implicit autoref in the unsafe model (#60)

Author: lucic71

cpp2rust/converter/converter.cpp

@@ -9,6 +9,7 @@
 #include <llvm/Support/ConvertUTF.h>
 
 #include <format>
+#include <utility>
 
 #include "compiler.h"
 #include "converter/converter_lib.h"
@@ -1059,7 +1060,10 @@ void Converter::ConvertLoopVariable(clang::VarDecl *decl,
       StrCat(std::format(".as_mut_ptr().add({})", loop_var_name));
     }
   } else {
-    Convert(range_init);
+    {
+      PushExplicitAutoref autoref(*this, /*is_mut=*/false);
+      Convert(range_init);
+    }
     StrCat(std::format("[{}]", loop_var_name));
     StrCat(".clone()");
   }
@@ -2218,19 +2222,25 @@ void Converter::EmitStmtExprTail(clang::Expr *tail) { Convert(tail); }
 bool Converter::VisitConditionalOperator(clang::ConditionalOperator *expr) {
   StrCat(keyword::kIf);
   ConvertCondition(expr->getCond());
+  bool branch_is_addr =
+      expr->isLValue() && !isRValue() && !expr->getType()->isFunctionType();
   {
     PushBrace then_brace(*this);
-    if (expr->isLValue() && !isRValue() && !expr->getType()->isFunctionType()) {
+    if (branch_is_addr) {
       StrCat(token::kRef, keyword_mut_);
     }
+    PushExplicitAutoref no_autoref(*this, branch_is_addr ? std::nullopt
+                                                         : autoref_mut_);
     Convert(expr->getTrueExpr());
   }
   StrCat(keyword::kElse);
   {
     PushBrace else_brace(*this);
-    if (expr->isLValue() && !isRValue() && !expr->getType()->isFunctionType()) {
+    if (branch_is_addr) {
       StrCat(token::kRef, keyword_mut_);
     }
+    PushExplicitAutoref no_autoref(*this, branch_is_addr ? std::nullopt
+                                                         : autoref_mut_);
     Convert(expr->getFalseExpr());
   }
   return false;
@@ -2285,11 +2295,7 @@ bool Converter::VisitDeclRefExpr(clang::DeclRefExpr *expr) {
 
   if (decl->getType()->getAs<clang::ReferenceType>() && !isAddrOf() &&
       !map_iter_decls_.contains(clang::dyn_cast<clang::VarDecl>(decl))) {
-    {
-      PushParen paren(*this);
-      StrCat(GetPointerDerefPrefix(decl->getType().getNonReferenceType()),
-             std::move(str));
-    }
+    EmitDeref(std::move(str), decl->getType().getNonReferenceType());
     SetValueFreshness(expr->getType());
     return false;
   }
@@ -2362,9 +2368,11 @@ bool Converter::ConvertCXXOperatorCallExpr(clang::CXXOperatorCallExpr *expr) {
       Convert(expr->getArg(0));
     }
     break;
-  case clang::OverloadedOperatorKind::OO_Subscript:
+  case clang::OverloadedOperatorKind::OO_Subscript: {
+    PushExplicitAutoref autoref(*this, IsMutatingCall(expr));
     ConvertArraySubscript(expr->getArg(0), expr->getArg(1), expr->getType());
     break;
+  }
   case clang::OverloadedOperatorKind::OO_LessLess:
     if (IsCallToOstream(expr)) {
       ConvertCallToOstream(expr);
@@ -2430,8 +2438,7 @@ bool Converter::VisitMemberExpr(clang::MemberExpr *expr) {
   }
 
   if (!isAddrOf() && member->getType()->isReferenceType()) {
-    PushParen paren(*this);
-    StrCat(GetPointerDerefPrefix(member->getType().getNonReferenceType()), str);
+    EmitDeref(std::move(str), member->getType().getNonReferenceType());
     return false;
   }
 
@@ -3200,10 +3207,14 @@ void Converter::ConvertPointerOffset(clang::Expr *base, clang::Expr *idx,
 
 void Converter::ConvertArraySubscript(clang::Expr *base, clang::Expr *idx,
                                       clang::QualType type) {
-  Convert(base->IgnoreImplicit());
   if (IsUniquePtr(base->getType())) {
+    PushExplicitAutoref no_autoref(*this, std::nullopt);
+    Convert(base->IgnoreImplicit());
     StrCat(".as_mut().unwrap()");
+  } else {
+    Convert(base->IgnoreImplicit());
   }
+  PushExplicitAutoref no_autoref(*this, std::nullopt);
   PushBracket bracket(*this);
   {
     PushParen paren(*this);
@@ -3509,11 +3520,19 @@ void Converter::ConvertAddrOf(clang::Expr *expr, clang::QualType pointer_type) {
   }
 }
 
+void Converter::EmitDeref(std::string inner, clang::QualType pointee_type) {
+  auto wrap = std::exchange(autoref_mut_, std::nullopt);
+  PushParen outer(*this, wrap.has_value());
+  if (wrap) {
+    StrCat(*wrap ? "&mut" : "&");
+  }
+  PushParen paren(*this);
+  StrCat(GetPointerDerefPrefix(pointee_type), std::move(inner));
+}
+
 void Converter::ConvertDeref(clang::Expr *expr) {
   if (!isAddrOf()) {
-    PushParen paren(*this);
-    StrCat(GetPointerDerefPrefix(expr->getType()->getPointeeType()),
-           ToString(expr));
+    EmitDeref(ToString(expr), expr->getType()->getPointeeType());
   } else {
     Convert(expr);
   }

cpp2rust/converter/converter.h

@@ -474,6 +474,8 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
 
   virtual void ConvertDeref(clang::Expr *expr);
 
+  void EmitDeref(std::string inner, clang::QualType pointee_type);
+
   virtual void ConvertArrow(clang::Expr *expr);
 
   virtual void ConvertCast(clang::QualType qual_type);
@@ -515,6 +517,17 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
   clang::ASTContext &ctx_;
   clang::FunctionDecl *curr_function_ = nullptr;
   bool in_function_formals_ = false;
+  std::optional<bool> autoref_mut_;
+
+  struct PushExplicitAutoref {
+    Converter &c;
+    std::optional<bool> prev;
+    PushExplicitAutoref(Converter &c, std::optional<bool> v)
+        : c(c), prev(c.autoref_mut_) {
+      c.autoref_mut_ = v;
+    }
+    ~PushExplicitAutoref() { c.autoref_mut_ = prev; }
+  };
   std::stack<clang::Expr *> curr_for_inc_;
   std::stack<clang::QualType> curr_init_type_;
 

cpp2rust/converter/converter_lib.cpp

@@ -156,6 +156,15 @@ bool IsMut(clang::QualType qual_type) {
            qual_type->getPointeeType().isConstQualified());
 }
 
+bool IsMutatingCall(const clang::CallExpr *expr) {
+  if (auto *callee = expr->getDirectCallee()) {
+    if (auto *method = clang::dyn_cast<clang::CXXMethodDecl>(callee)) {
+      return !method->isConst();
+    }
+  }
+  return true;
+}
+
 bool IsOverloadedFunction(const clang::FunctionDecl *decl) {
   const auto *ctx = decl->getDeclContext();
   const auto decl_name = decl->getDeclName();

cpp2rust/converter/converter_lib.h

@@ -45,6 +45,8 @@ bool IsUnsignedArithOp(const clang::BinaryOperator *expr);
 
 bool IsMut(clang::QualType qual_type);
 
+bool IsMutatingCall(const clang::CallExpr *expr);
+
 bool IsOverloadedFunction(const clang::FunctionDecl *decl);
 
 bool IsOverloadedMethod(const clang::CXXMethodDecl *decl);

tests/lit/lit/formats/Cpp2RustTest.py

@@ -29,7 +29,7 @@ def __init__(self):
     self.regex_translation_fail = re.compile(r"//\s*translation-fail\s*(?::\s*(.*))?$", re.MULTILINE)
     self.regex_nondet_result = re.compile(r"//\s*nondet-result\s*(?::\s*(.*))?$", re.MULTILINE)
     self.rust_version = read_rust_version()
-    os.environ['RUSTFLAGS'] = '-Awarnings -A dangerous-implicit-autorefs'
+    os.environ['RUSTFLAGS'] = '-Awarnings'
 
   def updateExpected(self, generated, expected_path):
     os.makedirs(os.path.dirname(expected_path), exist_ok=True)

tests/unit/implicit_autoref.cpp

@@ -0,0 +1,29 @@
+#include <cassert>
+#include <vector>
+
+struct Holder {
+  std::vector<int> v;
+};
+
+int main() {
+  std::vector<int> v;
+  v.push_back(10);
+  v.push_back(20);
+
+  std::vector<int> *p = &v;
+  int a = (*p)[0];
+  (*p)[1] = 30;
+
+  Holder h;
+  h.v.push_back(40);
+  h.v.push_back(50);
+  Holder *hp = &h;
+  int b = (*hp).v[0];
+  (*hp).v[1] = 60;
+
+  assert(a == 10);
+  assert((*p)[1] == 30);
+  assert(b == 40);
+  assert((*hp).v[1] == 60);
+  return 0;
+}

tests/unit/out/refcount/implicit_autoref.rs

@@ -0,0 +1,65 @@
+extern crate libcc2rs;
+use libcc2rs::*;
+use std::cell::RefCell;
+use std::collections::BTreeMap;
+use std::io::prelude::*;
+use std::io::{Read, Seek, Write};
+use std::os::fd::AsFd;
+use std::rc::{Rc, Weak};
+#[derive(Default)]
+pub struct Holder {
+    pub v: Value<Vec<i32>>,
+}
+impl Clone for Holder {
+    fn clone(&self) -> Self {
+        let mut this = Self {
+            v: Rc::new(RefCell::new((*self.v.borrow()).clone())),
+        };
+        this
+    }
+}
+impl ByteRepr for Holder {}
+pub fn main() {
+    std::process::exit(main_0());
+}
+fn main_0() -> i32 {
+    let v: Value<Vec<i32>> = Rc::new(RefCell::new(Vec::new()));
+    (*v.borrow_mut()).push(10);
+    (*v.borrow_mut()).push(20);
+    let p: Value<Ptr<Vec<i32>>> = Rc::new(RefCell::new((v.as_pointer())));
+    let a: Value<i32> = Rc::new(RefCell::new(
+        ((((*p.borrow()).to_strong().as_pointer()) as Ptr<i32>)
+            .offset(0_u64 as isize)
+            .read()),
+    ));
+    (((*p.borrow()).to_strong().as_pointer()) as Ptr<i32>)
+        .offset(1_u64 as isize)
+        .write(30);
+    let h: Value<Holder> = Rc::new(RefCell::new(<Holder>::default()));
+    (*(*h.borrow()).v.borrow_mut()).push(40);
+    (*(*h.borrow()).v.borrow_mut()).push(50);
+    let hp: Value<Ptr<Holder>> = Rc::new(RefCell::new((h.as_pointer())));
+    let b: Value<i32> = Rc::new(RefCell::new(
+        (((*(*hp.borrow()).upgrade().deref()).v.as_pointer() as Ptr<i32>)
+            .offset(0_u64 as isize)
+            .read()),
+    ));
+    ((*(*hp.borrow()).upgrade().deref()).v.as_pointer() as Ptr<i32>)
+        .offset(1_u64 as isize)
+        .write(60);
+    assert!(((*a.borrow()) == 10));
+    assert!(
+        (((((*p.borrow()).to_strong().as_pointer()) as Ptr<i32>)
+            .offset(1_u64 as isize)
+            .read())
+            == 30)
+    );
+    assert!(((*b.borrow()) == 40));
+    assert!(
+        ((((*(*hp.borrow()).upgrade().deref()).v.as_pointer() as Ptr<i32>)
+            .offset(1_u64 as isize)
+            .read())
+            == 60)
+    );
+    return 0;
+}

tests/unit/out/unsafe/implicit_autoref.rs

@@ -0,0 +1,37 @@
+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;
+#[repr(C)]
+#[derive(Clone, Default)]
+pub struct Holder {
+    pub v: Vec<i32>,
+}
+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.push(20);
+    let mut p: *mut Vec<i32> = (&mut v as *mut Vec<i32>);
+    let mut a: i32 = (&mut (*p))[(0_u64) as usize];
+    (&mut (*p))[(1_u64) as usize] = 30;
+    let mut h: Holder = <Holder>::default();
+    h.v.push(40);
+    h.v.push(50);
+    let mut hp: *mut Holder = (&mut h as *mut Holder);
+    let mut b: i32 = (&mut (*hp)).v[(0_u64) as usize];
+    (&mut (*hp)).v[(1_u64) as usize] = 60;
+    assert!(((a) == (10)));
+    assert!((((&mut (*p))[(1_u64) as usize]) == (30)));
+    assert!(((b) == (40)));
+    assert!((((&mut (*hp)).v[(1_u64) as usize]) == (60)));
+    return 0;
+}

tests/unit/out/unsafe/vector2.rs

@@ -14,15 +14,15 @@ pub unsafe fn fn_0(v: *mut Vec<i32>, mut v3: Vec<i32>) {
     v2.push(0);
     v2.push(1);
     v2.push(3);
-    x = (*v)[(2_u64) as usize];
+    x = (&mut (*v))[(2_u64) as usize];
     v2[(0_u64) as usize] = 1;
     (if true { &mut v3 } else { &mut (*v) })[(0_u64) as usize] = 7;
     v2 = (*v).clone();
-    (*v4)[(1_u64) as usize] = 13;
+    (&mut (*v4))[(1_u64) as usize] = 13;
     assert!(((x) == (6)));
     assert!(((*((*v).first_mut().unwrap())) == (4)));
-    assert!((((*v)[(1_u64) as usize]) == (5)));
-    assert!((((*v)[(2_u64) as usize]) == (6)));
+    assert!((((&mut (*v))[(1_u64) as usize]) == (5)));
+    assert!((((&mut (*v))[(2_u64) as usize]) == (6)));
     assert!(((*((*v).last_mut().unwrap())) == (20)));
     assert!(((v2[(0_u64) as usize]) == (4)));
     assert!(((v2[(1_u64) as usize]) == (5)));

tests/unit/out/unsafe/vector3.rs

@@ -35,7 +35,7 @@ unsafe fn main_0() -> i32 {
     'loop_: for v2 in 0..(v.len()) {
         let mut v2 = v.as_mut_ptr().add(v2);
         'loop_: for i in 0..((*v2).len()) {
-            let mut i = (*v2)[i].clone();
+            let mut i = (&(*v2))[i].clone();
             printf(b"%d\n\0".as_ptr() as *const i8, ((i) + (3)));
         }
     }