Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation of asynchronous self-calls through let is broken #41

Closed
TobiasWrigstad opened this issue Dec 8, 2014 · 7 comments
Closed

Comments

@TobiasWrigstad
Copy link
Contributor

Summary: when compiling let this = that in that.foo(), the type of this (in the C-level) should be changed from the "internal type" to the "external type" (aka. pony_actor_t*).

The long of it

The following Encore code uses the "standard let-binding trick" to achieve an asynchronous call on this (last line, including all code for completeness):

class StreamFiller 
  def computeStream(state:int) : E 
    let 
      res = new E 
    in 
      if (state > 110) then 
        res.eos()
      else 
        res.init(state, let that = this in that.computeStream(state + 1))      

However, the method gets compiled thus (se my XXX marking below to spot the error):

E_data* StreamFiller_computeStream(StreamFiller_data* this, int64_t state)
{
  E_data* _tmp0 = pony_alloc(sizeof(E_data));;
  memset(_tmp0, 0, sizeof(E_data));;;
  E_data* _tmp1 = _tmp0;;
  E_data* _tmp2_ite;
  if (({;
        int64_t _tmp3 = 110;;
        int64_t _tmp4 = (state > _tmp3);;; _tmp4;}))
  {
    ;
    E_data* _tmp5 = E_eos(_tmp1);;;
    _tmp2_ite = _tmp5;;;
  }
  else
  {
    ;
    E_data* _tmp12 = E_init(_tmp1, ({ state;}), ({;
                              pony_actor_t* _tmp6 = this;; // XXXXXXXXXXXXXXX
                              future_t* _tmp10 = future_mk();;
                              pony_arg_t (_tmp11[2]) = {{.p = _tmp10}, {.i = ({;
                                                                    int64_t _tmp8 = 1;;
                                                                    int64_t _tmp9 = (state + _tmp8);;; _tmp9;})}};;
                              pony_sendv(({ _tmp6;}), MSG_StreamFiller_computeStream, 2, _tmp11);;; _tmp10;}));;;
    _tmp2_ite = _tmp12;;;
  };;;
  return _tmp2_ite;;
}

The problem here is that this has two different types (at the C level). When we interact with this as an actor, we use the pony_actor_t type, but normally we want the StreamFiller_data type that allows field access and synchronous method calls.

E_data* StreamFiller_computeStream(StreamFiller_data* this, int64_t state)
{
  E_data* _tmp0 = pony_alloc(sizeof(E_data));;
  memset(_tmp0, 0, sizeof(E_data));;;
  E_data* _tmp1 = _tmp0;;
  E_data* _tmp2_ite;
  if (({;
        int64_t _tmp3 = 110;;
        int64_t _tmp4 = (state > _tmp3);;; _tmp4;}))
  {
    ;
    E_data* _tmp5 = E_eos(_tmp1);;;
    _tmp2_ite = _tmp5;;;
  }
  else
  {
    ;
    E_data* _tmp12 = E_init(_tmp1, ({ state;}), ({;
                              pony_actor_t* _tmp6 = this->aref;; // XXXX FIXED!
                              future_t* _tmp10 = future_mk();;
                              pony_arg_t (_tmp11[2]) = {{.p = _tmp10}, {.i = ({;
                                                                    int64_t _tmp8 = 1;;
                                                                    int64_t _tmp9 = (state + _tmp8);;; _tmp9;})}};;
                              pony_sendv(({ _tmp6;}), MSG_StreamFiller_computeStream, 2, _tmp11);;; _tmp10;}));;;
    _tmp2_ite = _tmp12;;;
  };;;
  return _tmp2_ite;;
}
@TobiasWrigstad
Copy link
Contributor Author

In hindsight, this seems to be a duplicate of #18.

@EliasC
Copy link
Contributor

EliasC commented Dec 8, 2014

In hindsight, this seems to be a duplicate of #18.

Not really, since #18 is about when fields are captured in a closure (i.e. should x.f in a closure capture x and at call time lookup f, or capture x.f directly).

@EliasC
Copy link
Contributor

EliasC commented Dec 8, 2014

Related however is the discussion in #40 on whether or not we should try to disallow asynchronous calls on (aliases of) this. Right now, the code let this = that in get that.foo() would deadlock, which is bad. @ebjohnsen has suggested await-semantics, which would fix the problem (this could be both explicit, or implicit on calls to this). If we want to disallow asynchronous calls on this we could try to track the this-type when assigning from this.

@TobiasWrigstad
Copy link
Contributor Author

In hindsight, this seems to be a duplicate of #18.

Not really, since #18 is about when fields are captured in a closure (i.e. should x.f in a closure capture > x and at call time lookup f, or capture x.f directly).

I disagree. On the implementation level it seems to be very related. The common problem (which is not the full story in either case) seems to be that there are two types of this -- the internal and the external types -- and that we seem to not be 100% clear on when we should use which type.

@EliasC
Copy link
Contributor

EliasC commented Dec 8, 2014

I disagree. On the implementation level it seems to be very related.

Related, yes. Duplicate, no.

@TobiasWrigstad
Copy link
Contributor Author

I disagree. On the implementation level it seems to be very related.

Related, yes. Duplicate, no.

+1

@EliasC
Copy link
Contributor

EliasC commented Dec 10, 2014

Fixed as of 9f917d2. The following code now works. This should allow @supercooldave to implement streams the way he wanted in the first place. This code works:

class Foo
  def fr0b() : bool
    true

  def foo() : Fut bool
    let that = this in{
      that.fr0b();
    }

  def bar() : Fut bool
    let that = null : Foo in{
      that = this;
      that.fr0b()
    }

  def baz() : Fut bool
    let id = \ (x : a) -> x
        that = id(this)
    in
      that.fr0b()

class Main
  def main() : void
    let x = new Foo in{
      assertTrue(get get x.foo());
      assertTrue(get get x.bar());
      assertTrue(get get x.baz());
      print "Done!";

@EliasC EliasC closed this as completed Dec 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants