[BUG] Confusing behavior with structs and closures

[lsh]

Hi, last night I was playing around in the notebook and ran into a few interesting sharp edges.

The struct I had in mind was a D3 inspired "line builder", where it takes an x and y function, and some points, and returns the transformed points with that function.

The builder was defined as follows:

from Vector import DynamicVector
from DType import DType

alias Vec2 = SIMD[DType.f32, 2]

fn constant(x: F32) -> F32:
    return x

struct Line:
    var x: fn(F32) -> F32
    var y: fn(F32) -> F32
    
    fn __init__(self&):
        self.x = constant
        self.y = constant
        
    fn set_x(self&, x: fn(F32) -> F32):
        self.x = x
        
    fn set_y(self&, y: fn(F32) -> F32):
        self.x = y
        
    fn evaluate(self, points: DynamicVector[Vec2]) -> DynamicVector[Vec2]:
        var out = DynamicVector[Vec2]()
        for i in range(points.__len__()):
            let p = points[i]
            let x = p[0]
            let y = p[1]
            out.push_back(Vec2(self.x(x),self.y(y)))
        return out

The goal was to follow something like the builder idiom. For the following here is some book keeping:

# no lambdas for now so a custom x here
fn my_x(x: F32) -> F32:
    return x*x

points.push_back(Vec2(0,0))
points.push_back(Vec2(1,0))
points.push_back(Vec2(2,2))
points.push_back(Vec2(3,1))

Originally I wanted to write

let e = Line().set_x(my_x).set_y(constant).evaluate(points)

However that errors with:

error: Expression [7]:23:25: invalid call to 'set_x': invalid use of mutating method on rvalue of type 'Line'
    let e = Line().set_x(my_x).set_y(constant).evaluate(points)
            ~~~~~~~~~~~~^~~~~~

Expression [5]:13:5: function declared here
    fn set_x(self&, x: fn(F32) -> F32):
    ^

So I wrote it out as:

var l = Line()
l.set_x(my_x)
l.set_y(constant)
let e = l.evaluate(points)

Here is the second sharp edge
(see #39 about the e.__len__())

for i in range(e.__len__()):
    print(e[i])

outputs:

[0.000000, 0.000000]
[1.000000, 0.000000]
[2.000000, 2.000000]
[3.000000, 1.000000]

In other words, evaluate is still using the original self.x, not the current self.x.

[lsh]

This also brings up the question: how are closures stored on a struct? In Rust, one would likely Box the closure. Would this allocate in Mojo to store the closure?

[abduld]

This is an interesting and useful pattern. We will make sure we have an example of how to do this in Mojo ASAP

[abduld]

The basic idea is that your set_x and set_y need to return a new instance of the class, since you are chaining the method calls. Try

struct Line:
    var x: fn (F32) -> F32
    var y: fn (F32) -> F32

    fn __init__(inout self):
        self.x = constant
        self.y = constant

    fn __init__(inout self, x: fn (F32) -> F32, y: fn (F32) -> F32):
        self.x = x
        self.y = y

    fn set_x(self, x: fn (F32) -> F32) -> Self:
        return Self(x, self.y)

    fn set_y(self, y: fn (F32) -> F32) -> Self:
        return Self(self.x, y)

    fn evaluate(self, points: DynamicVector[Vec2]) -> DynamicVector[Vec2]:
        var out = DynamicVector[Vec2]()
        for i in range(points.__len__()):
            let p = points[i]
            let x = p[0]
            let y = p[1]
            out.push_back(Vec2(self.x(x), self.y(y)))
        return out

which should allow you to write

    let e = Line().set_x(my_x).set_y(constant).evaluate(points)

[lsh]

Thanks for looking into it! That makes sense. I wonder if there could be a self^ which moves self into a method

[nmsmith]

Why not allow r-values to be mutated in this way? Is it not possible to come up with a semantics for this?

Oh wait, I see: the original definition of set_x isn't returning anything.

I don't understand why set_x can't be written as follows, though:

fn set_x(inout self, x: fn (F32) -> F32) -> Self:
    self.x = x
    return self

[sa-]

The basic idea is that your set_x and set_y need to return a new instance

This is a lot like scala's copy function on a case class which creates a shallow copy.

See "Copying" for more details here: https://docs.scala-lang.org/tour/case-classes.html

[lsh]

I was trying to trigger moves rather than copies in this case, but I'm not sure that's possible right now

[Mogball]

@lsh there's a typo in your code

    fn set_y(self&, y: fn(F32) -> F32):
        self.x = y

[Mogball]

Closing this for now.