AST-27 Desugar objects and functions into let

[rossberg]

Remove binding identifiers from Obj and Func forms and desugar them into lets:

• Make let return its RHS value
• In AST, change FuncD to FuncE and remove id from ObjE
• In parser, desugar function and object forms into let or expression forms
• Remove previous grammar hacks for dealing with objects
• In definedness check, allow delayed use of free variables in actor
• Temporary desugaring hacks turning let x = actor back into actor x in IR
• Temporary desugaring hack turning FuncE back into FuncE in IR

FuncE still carries a name, but this is only used for tracing outputs, it is no longer semantically relevant.

TODO(joachim): replace FuncD with FuncE in IR and compiler (I tried, but did not understand code generation well enough)
TODO(joachim): handle free and forward references in actors

[rossberg]

Note that it's not necessary for Ir.LetD to return values, since that is taken care fo by desugaring. After FuncD is replaced with FuncE in the IR, all non-ExpD decs should have type () there.

[nomeata]

Hello from above the Caribbeans. We might have a race going on with #189 now :-)

[nomeata]

Although, this seems to be only affecting the front-end until IR. Nice decoupling :-)

[nomeata]

In definedness check, allow delayed use of free variables in actor

Are you sure we will be able to support that? My current implementation strategy for closed actors is “serialize upon actor initialization”, which means that everything needs to be defined, with a special case for actors.

[nomeata]

I have qualms about the changes to the definedness checker. In any case, instead of deleting test cases, just move them to run if they ought to work now.

[nomeata]

I suggest you leave in your change to definedness.ml (to make it permissive enough for your testing, although too permissive for what the backend will be able to handle), but with a TODO around ObjE indicating that Actors needs special handling, and you leave in the test case (with updated output).

I will then deal with it together with AST-49 (closing over values in actors).

[nomeata]

Ah, you raised that point on JIRA. Let’s continue here, ok? You wrote

Joachim Breitner, at which point would you send it? At that point you need to have all necessary values available. They may come from outer scopes, so the init may need to be hoisted to an outer scope. Consider:

{
  let h = {
    actor a {f() = x; g() = y};
    let x = 0;
    func h() : async Nat { (await a.f()) + (await a.g()) }
  }
  let y = 0;
  // This is the earliest point to init a, which isn’t even in scope here
  h();
}

Also, what about async code? Will it be a problem when there is an await between the actor and its init point (I couldn’t find any, but it may be subtle).

[nomeata]

So, the design I had in my mind would

• At the beginning of a block, instantiate all declared actors (but not initialize them yet). This means that an actorref is valid everywhere in a block.
• When we come across actor { … }, we collect its environment, serialize it, and send it to a special init method.

This design does not allow actors to capture values that are defined later other than actorrefs (and I expect that you have no great sympathy for such discontinuities). But the bigger problem is: This only works with ActorD declarations, but not with ActorE expressions, as with expressions we don't know which actors are declared where.

So the only alternative I see right now is:

• When we come across actor { … }, instantiate the module, which creates the actorref, and collect its environment. But do not serialize it yet. Instead, put the actorref and the closure it in a RTS-managed list of actors-to-be-instantiated.
• At the end of handling the current message (e.g. before Garbage Collection), go through the queue, serialize all the closures, and sends it to each actor’s init method.

By that time, all definitions from any recursive declaration group have been filled in.

This has the problem that in the case of actor{ …}.foo() we probably want to call the init function before we call foo(). Two alternatives:

• We expect the hypervisor to reorder the messages for us. More precisely, the hypervisor knows about the special meaning of the init method, treats freshly instantiated actors as uninitialized and holds off calling any other methods on them (keeps them in the queue) until it has an init message to deliver. This has the advantage that it avoid races when three canisters are involved.
• We queue all message sends withing one message execution, in order, and send all of them at the end of the current message execution. This is semantically not observerable, i.e. we just queue them inside Wasm instead of in the Hypervisor bookkeeping for this actor.

[rossberg]

@nomeata:

This only works with ActorD declarations, but not with ActorE expressions.

What's the difference, given that they are interchangeable modulo an auxiliary BlockE or LetD?

put the actorref and the closure it in a RTS-managed list of actors-to-be-instantiated.

Yes, that was my conclusion as well. Especially since you can create a dynamic number of actors, e.g., in a loop. (Though in this case I couldn't find an example where either the actor is never used, or there is a local eager use in that loop.)

At the end of handling the current message (e.g. before Garbage Collection), go through the queue, serialize all the closures, and sends it to each actor’s init method.

I'm not sure "end of current message" is late enough for async. Consider:

func meth() : async T {
  actor a { f() { ...b... };
  await something();
  actor b { g() { ...a... };
}

This has the problem that in the case of actor{ …}.foo() we probably want to call the init function before we call foo().

Right. I think the more uniform approach is to invoke init right before the point where the actor is first "eagerfied", either directly or transitively. The definedness check should imply that at this point (1) all references are available, while (2) nobody has used it yet.

This requires no RTS-managed queue, only some syntactic transformations to make the necessary values accessible at the right point -- e.g. as a closure build locally but invoked at the aforementioned point. It should be compatible with both async and with dynamically created actors. Unused actors (that are never eagerified) would never be initialised, but that's fine because they are dead code.

More precisely, the hypervisor knows about the special meaning of the init method

Yeah, would be great if we could avoid such a special mechanism in the OS.

We queue all message sends withing one message execution, in order, and send all of them at the end of the current message execution.

Maybe I'm misunderstanding this, but isn't that the Dfn semantics already?

Just for the sake of concrete example of the message ordering problem, let's take this minimal piece:

actor A {
  func meth() {
    actor B {f() { ...x...} };
    let x = ...;
    // (*) B.init(x);
    C.send(B);
  }
}

At point (*), A would implicitly send the init message to B. It would then send a message to C passing along B. After that, C could send f to B.

The problem is that there is no guarantee that B receives init before f. This is exactly the transitive message ordering problem. We'd need some kind of barrier mechanism.

Would it work to make the init message return an async (), so that we can synchronise on it? Then the above could be transformed into s.th like

actor A {
  func meth() {
    actor B {f() { ...x...} };
    let x = ...;
    // (*) let ready = B.init(x);
    ignore(async { await ready; C.send(B) });
  }
}

Of course, when meth sends multiple messages this gets slightly more tricky, since we still need to maintain their order, so they cannot be put into separate asyncs. Hm...

[nomeata]

This only works with ActorD declarations, but not with ActorE expressions.

What's the difference, given that they are interchangeable modulo an auxiliary BlockE or LetD?

With ActorD, at the beginning of the block (i.e. the full scope of the actorref) I know statically which modules (i.e. actor sans closure) will be created, and I can instantiate these modules, which gives me a valid actorref.

With ActorE I can have

{ let a = actor { … b … };
  let b = if long computation then actor { … } else actor { … };
}

I cannot instantiate b before the first line, because I cannot know which of the two branches are taken.

Note that you cannot express that code directly with ActorD.

I'm not sure "end of current message" is late enough for async.

Ugh, async. I don’t think about async, it seems, it has been translated away already when I start to come in :-)

I think the more uniform approach is to invoke init right before the point where the actor is first "eagerfied", either directly or transitively. The definedness check should imply that at this point (1) all references are available, while (2) nobody has used it yet.

That sounds promising. Will think about it on the next flight.

Unused actors (that are never eagerified) would never be initialised, but that's fine because they are dead code.

Is that sound? Actors can have side-effects (e.g. call further actors during their initialization).

We queue all message sends withing one message execution, in order, and send all of them at the end of the current message execution.

Maybe I'm misunderstanding this, but isn't that the Dfn semantics already?

Precisely, but currently the hypervisor maintains that queue; with this change we might have to maintain another queue in the RTS. Also, we could then re-order the message to put all init calls first.

Would it work to make the init message return an async (), so that we can synchronise on it? Then the above could be transformed into s.th like

Would this restrict actor creation to contexts where an await would be valid?

Boarding flight to Zurich now.

[rossberg]

@nomeata:

Note that you cannot express that code directly with ActorD.

I don't see why not. How would

{ actor a { … b … };
  let b = if long computation then { actor b { … } } else { actor b { … } }
}

be any different?

Is that sound? Actors can have side-effects (e.g. call further actors during their initialization).

Ah, good point. So there would need to be a special case for actors that are never forced explicitly.

with this change we might have to maintain another queue in the RTS.

But why would this be necessary?

Would this restrict actor creation to contexts where an await would be valid?

No, see my example. Only message sends would need async, but you can always locally introduce async expressions. That's not observable for (individual) message sends, since they are async anyway (though the message ordering guarantee with multiple sends remains trickier).

I should also mention that we probably don't need to solve the general problem for the pre release. For that, only actors need to work.

[nomeata]

Note that you cannot express that code directly with ActorD.

I don't see why not. How would

{ actor a { … b … };
  let b = if long computation then { actor b { … } } else { actor b { … } }
}

be any different?

Ah, I see: I was not considering the nested-block scenario, and the solution in the back of my head would look at { actor b { … } } and consider the escaping reference to the actor as a first-class, eager use of it (as it was the case for an earlier design of the definedness checker, for closures). But you are right that we are already more ambitious than that and have a definedness checker that can handle such nested blocks, so we should do it for blocks as well.

tl;dr: You are right, that should be equivalent.

with this change we might have to maintain another queue in the RTS.

But why would this be necessary?

If we hold off sending the init messages in the RTS, we want to hold off sending all messages, to send them in order. (This was with the flush-at-end approach; your flush-at-first-use approach might not need a queue.)

I still hope we can do something better than such a heavy synchronization protocol. How about the variant where the receiving actor queues incoming message until it has seen the init message?

[nomeata]

Even with local actors out-of-scope for the preview release, this is still useful (and the only pain points are around local actors). And we should also not waste work, so I propose we invest enough work to merge this (and #189) without breaking too much existing support for actors.

With Andreas being away for a while, I suggest I take over this PR, address my own concerns, and then get it merged.

[nomeata]

Ok, I updated the branch to address my own nits, so I would accept it now. Should I just merge, or do we want another review? (@crusso maybe)

[nomeata]

I also did the corresponding FuncE introduction in IR. There were some nasty bugs on the way, but here it is: #196 (that PR is relative to this one).

[crusso]

Is this deliberate? Joachim, have you changed the IR letD to also return the value of rhs?This contradics Andreas first comment on the PR:

" Note that it's not necessary for Ir.LetD to return values, since that is taken care fo by desugaring. After FuncD is replaced with FuncE in the IR, all non-ExpD decs should have type () there"

[nomeata]

Oh, I guess I was confused when I wrote that. But I am fixing this in #196 in any case (including Check_ir), so it’s fine for now if we merge both.

In fact, I want to refactor BlockE further to (dec list * exp), and then declarations have no type at all (which means less to check in Check_ir and worry about in Construct).

[crusso]

Both sound good

[crusso]

What if ds is [], won't split_last fail? Do we rule out empty blocks earlier? Even if so, maybe add an assert...

[crusso]

Never mind - the diff just doesn't show a previous S.Block [] case...

[nomeata]

In fact, in #196 Check_ir asserts that the list is non-empty, the afore-mentioned refactoring will make that explicit on the type level.

[crusso]

Good.

[crusso]

do we know that is going on here?

[crusso]

This must be [AST-60].

[nomeata]

Yes, this is unrelated to this change (just the error message looks different now).

[crusso]

are anonymous functions not allowed anymore? Or is this just a trivial change?

[nomeata]

This was done by Andreas, I don’t know why.

[crusso]

Ok.