From 94eba522c020eb6c75a91900382af85b9fbafee5 Mon Sep 17 00:00:00 2001
From: Daniel Falbel <dfalbel@gmail.com>
Date: Wed, 27 Nov 2024 11:04:28 -0300
Subject: [PATCH] Switch to RObject ref

---
 crates/ark/src/variables/variable.rs | 12 ++++++------
 crates/harp/src/environment.rs       |  6 +++---
 crates/harp/src/utils.rs             | 12 +++++++-----
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/crates/ark/src/variables/variable.rs b/crates/ark/src/variables/variable.rs
index 6f06501d7..84d7c9e5a 100644
--- a/crates/ark/src/variables/variable.rs
+++ b/crates/ark/src/variables/variable.rs
@@ -1865,7 +1865,7 @@ mod tests {
     fn inspect_from_expr(code: &str) -> Vec<Variable> {
         let env = Environment::new(harp::parse_eval_base("new.env(parent = emptyenv())").unwrap());
         let value = harp::parse_eval_base(code).unwrap();
-        env.bind("x".into(), value.sexp);
+        env.bind("x".into(), &value);
         // Inspect the S4 object
         let path = vec![String::from("x")];
         PositronVariable::inspect(env.into(), &path).unwrap()
@@ -1973,7 +1973,7 @@ mod tests {
         r_task(|| {
             let env = Environment::new_empty().unwrap();
             let value = harp::parse_eval_base("matrix(0, nrow = 10000, ncol = 10000)").unwrap();
-            env.bind("x".into(), value.sexp);
+            env.bind("x".into(), &value);
 
             // Inspect the matrix, we should see the list of columns truncated
             let path = vec![String::from("x")];
@@ -1993,7 +1993,7 @@ mod tests {
         r_task(|| {
             let env = Environment::new_empty().unwrap();
             let value = harp::parse_eval_base("paste(1:5e6, collapse = ' - ')").unwrap();
-            env.bind("x".into(), value.sexp);
+            env.bind("x".into(), &value);
 
             let path = vec![];
             let vars = PositronVariable::inspect(env.into(), &path).unwrap();
@@ -2004,7 +2004,7 @@ mod tests {
             // Test for the empty string
             let env = Environment::new_empty().unwrap();
             let value = harp::parse_eval_base("''").unwrap();
-            env.bind("x".into(), value.sexp);
+            env.bind("x".into(), &value);
 
             let path = vec![];
             let vars = PositronVariable::inspect(env.into(), &path).unwrap();
@@ -2014,7 +2014,7 @@ mod tests {
             // Test for the single elment matrix, but with a large character
             let env = Environment::new_empty().unwrap();
             let value = harp::parse_eval_base("matrix(paste(1:5e6, collapse = ' - '))").unwrap();
-            env.bind("x".into(), value.sexp);
+            env.bind("x".into(), &value);
             let path = vec![];
             let vars = PositronVariable::inspect(env.into(), &path).unwrap();
             assert_eq!(vars.len(), 1);
@@ -2024,7 +2024,7 @@ mod tests {
             // Test for the empty matrix
             let env = Environment::new_empty().unwrap();
             let value = harp::parse_eval_base("matrix(NA, ncol = 0, nrow = 0)").unwrap();
-            env.bind("x".into(), value.sexp);
+            env.bind("x".into(), &value);
             let path = vec![];
             let vars = PositronVariable::inspect(env.into(), &path).unwrap();
             assert_eq!(vars.len(), 1);
diff --git a/crates/harp/src/environment.rs b/crates/harp/src/environment.rs
index 0e6a907de..238a5f507 100644
--- a/crates/harp/src/environment.rs
+++ b/crates/harp/src/environment.rs
@@ -91,13 +91,13 @@ impl Environment {
         std::iter::successors(Some(self.clone()), |p| p.parent())
     }
 
-    pub fn bind(&self, name: RSymbol, value: SEXP) {
+    pub fn bind(&self, name: RSymbol, value: &RObject) {
         unsafe {
-            Rf_defineVar(name.sexp, value.into(), self.inner.sexp);
+            Rf_defineVar(name.sexp, value.sexp, self.inner.sexp);
         }
     }
 
-    pub fn force_bind(&self, name: RSymbol, value: SEXP) {
+    pub fn force_bind(&self, name: RSymbol, value: &RObject) {
         let locked = self.is_locked_binding(name);
         if locked {
             self.unlock_binding(name);
diff --git a/crates/harp/src/utils.rs b/crates/harp/src/utils.rs
index 04cd88987..cd0e00f87 100644
--- a/crates/harp/src/utils.rs
+++ b/crates/harp/src/utils.rs
@@ -673,8 +673,9 @@ pub fn save_rds(x: SEXP, path: &str) {
     let path = RObject::from(path);
 
     let env = Environment::new(harp::parse_eval_base("new.env()").unwrap());
-    env.bind("x".into(), x);
-    env.bind("path".into(), path.sexp);
+    let x = RObject::from(x);
+    env.bind("x".into(), &x);
+    env.bind("path".into(), &path);
 
     let res = harp::parse_eval0("base::saveRDS(x, path)", env);
 
@@ -701,9 +702,10 @@ pub fn push_rds(x: SEXP, path: &str, context: &str) {
 
     let env = Environment::new(harp::parse_eval_base("new.env()").unwrap());
 
-    env.bind("x".into(), x);
-    env.bind("path".into(), path.sexp);
-    env.bind("context".into(), context.sexp);
+    let x = RObject::from(x);
+    env.bind("x".into(), &x);
+    env.bind("path".into(), &path);
+    env.bind("context".into(), &context);
 
     let res = harp::parse_eval0(".ps.internal(push_rds(x, path, context))", env);