wing icon indicating copy to clipboard operation
wing copied to clipboard

chore(compiler): remove `this` first arg from type system and implement some other instance binding mechanism

Open yoav-steinberg opened this issue 1 year ago • 4 comments

Wing's type system adds this as a first arg to all instance methods. I originally designed it that way almost automatically just because it fest obvious to me that method instances need some way of knowing what this is. An implication of this approach is that you can also treat instance methods as static methods with another argument:

a.do(1);
A.do(a,1); // Another way to do the same

But this approach of "how to tell a method what its instance is" makes it complex to pass an instance method as an argument expecting a function signature:

call_a_callback(callback: (x:num)) { callback(5); }
let f = print_num(x: num) => { print(x); }
call_a_callback(f); // This will work and print 5, no instance no cry
call_a_callback(a.do); // Does this compile? if the signature of `do` has a `this` then this won't compile

Another problem with this approach is although A.do(a,1) technically works, no one ever uses this.

I suggest removing this as first arg from the type system. We then have two options how to handle passing methods as an argument expecting a function signature:

  1. Now that we have static support we can easily distinguish between functions and methods and just fail if we try to pass a method to a function signature. Doing so is usually bad design because if we want some callback on an object, we can make the function expect an interface instead and make sure our object implements that interface.
  2. We can implement something like python's callable model where there's a hidden instance optionally attached to function signature argument. In TypeScript this is called binding an object to a function. In TypeScript you can do something like call_a_callback(a.do.bind(a));. We can make sure when instance methods are called we compile to that and maybe we'll also need to add something to our internal type system (FunctionSignature) to indicate whether this is a bound function (method) or not.

yoav-steinberg avatar Feb 27 '23 20:02 yoav-steinberg

#1942 addresses this partially by remodeling the "this" type as a separate field on the compiler's FunctionSignature struct instead of as a parameter. However this type information isn't used yet, so it's possible to encounter errors in generated JS code. For example, in Wing 0.8.26 this example fails during preflight:

resource A {
  var x: num;
  init() {
    this.x = 5;
  }

  f(inner: (): str) {
    let prev_x = this.x;
    inner();
    assert(this.x == prev_x);
  }

  g(): str {
    this.x = this.x + 1;
    return "done";
  }
}

let a = new A();
let b = a.g;
a.f(b);

Error message:

preflight error: Cannot read properties of undefined (reading 'x')

  note: intermediate javascript code:
      g()  {
        {
>>     this.x = (this.x + 1);
       return "done";
     }

Instead, it should probably fail during compilation, or we consider if there is a way to make this work reliably (can we do a.g.bind(a) in the generated JavaScript?)

Chriscbr avatar Apr 04 '23 15:04 Chriscbr

I suggest to fix the function signature comparison code so it doesn't equate signatures with differing this_types. This mean in our language you can't assign an instance method to a function type. Alternatively, if we want our language to support this, then we need to generate the js bind call in the assignment (or arg passing).

yoav-steinberg avatar Apr 04 '23 16:04 yoav-steinberg

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

github-actions[bot] avatar Jun 04 '23 06:06 github-actions[bot]

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

github-actions[bot] avatar May 07 '24 06:05 github-actions[bot]