dgkf / R

An experimental reimagining of R
https://dgkf.github.io/R
GNU General Public License v3.0
136 stars 5 forks source link

Bug when mutating promise #136

Closed sebffischer closed 3 months ago

sebffischer commented 5 months ago

It is not clear to me (yet) why this happens

inc = fn(x) {
  x[1] = x[1] + 1 
  x
}
add_two = fn(x) {
  inc(inc(x))
}
add_two(1)

Error: Internal Error (src/lang.rs:120)
backtrace:
1: add_two(1) => None
2: inc(inc(x)) => None
3: x[1] <- x[1] + 1 => None
4: x[1] + 1 => None
5: x[1] => None
sebffischer commented 5 months ago

I stumbled upon this when writing another test for the mutability of vectors, but this also fails in main

    #[test]
    fn nested_promises_can_be_mutated() {
        r_expect! {{"
            inc = fn(x) {
              x[1] = x[1] + 1 
              x
            }
            add_two = fn(x) {
              inc(inc(x))
            }
            add_two(1) == 3
         "}}
    }
sebffischer commented 5 months ago

So I think the bug here is that try_get and try_get_inner don't cover the Obj::Promise variant. I will try to fix it.

https://github.com/dgkf/R/blob/8fbbae8e289fb3218c6c1da1d6e4e23be05efae2/src/lang.rs#L260-L275

sebffischer commented 5 months ago

I think this means that we might need to wrap a promise in a RefCell, similiar to how we discussed it here: https://github.com/dgkf/R/pull/112 for the vector.

The problem is essentially the same, a promise has two states: unevaluated and evaluated. When e.g. try_get is called on a promise, it internally needs to evaluate the promise. But we then also want to keep the results of this evaluation and don't throw it away.

For this, we can implement something like:

struct Promise(RefCell<PromiseState>);

enum PromiseState {
  Evaluated(Obj, Obj::Expression),
  Unevaluated(Obj::Expression, Rc<Environment>)
}

like for the vector for the different RepTypes for Vector, we can then modify Promises in-place when we evaluate them.

Another question I have about promises is why do they keep the expression / environment that produced them even after being evaluated? Are both necessary, and if so, for what?

sebffischer commented 5 months ago

the question is when an evaluated promise is converted into its actual value. this should happen upon returning the promise or when passing the promise as an argument to another function I believe.

alternatively this happens when the promise is evaluated. this means that any metaprogramming such as substitute needs to be called at the beginning of the function. If this is the case then the enum that I suggested is also not needed, because a promise can never be in an evaluated state.

dgkf commented 5 months ago

a promise has two states: unevaluated and evaluated. When e.g. try_get is called on a promise, it internally needs to evaluate the promise. But we then also want to keep the results of this evaluation and don't throw it away.

This is currently tracked within the Obj::Promise - both the evaluated and unevaluated forms are stored simultaneously.

https://github.com/dgkf/R/blob/8fbbae8e289fb3218c6c1da1d6e4e23be05efae2/src/object/core.rs#L19

The promise starts as a Promise<None, Expr, Rc<Environment>>. Once evaluated, the value becomes a Pomise<Some<Box<Obj>>, Expr, Rc<Environment>>

The problem is essentially the same [as mutable vectors]

Although I do think that the solution in your PR probably needs to be applied more holistically to objects in general, this might not require it. In R, if you mutate a value it stops being a promise and is converted into a value.

> f <- function(x) {
+   print(force(x))
+   x[1] <- x[1] + 1
+   print(x)
+   print(substitute(x))
+ }
> f(c(1, 2, 3))
[1] 1 2 3   # force(x)
[1] 2 2 3   # x (after mutation)
[1] 2 2 3   # substitute(x), no longer a promise with an expression