Extend automatic receiver recovery to include arguments as recovery candidates.

[jemc]

In this week's sync call, @Praetonus mentioned that the compiler is missing out on recognizing some constructs as safe.

Particularly, he noted that the current rule for automatic receiver recovery is more restrictive than it needs to be.

The gist of the current rule is that we can automatically recover the receiver if all the arguments and the return value are sendable. @Praetonus mentioned that this rule could be tweaked to allow recovering one of the arguments instead, if the receiver and all other arguments are sendable. This makes sense in the context of thinking about the receiver in Pony as being just another argument to the function.

The purpose of this ticket is to discuss and track work for that rule enhancement.

[jemc]

@jasoncarr0 says there may be issues evaluation order of the receiver argument vs the other arguments, and the causing of invalidation.

[jasoncarr0]

Example where there's an issue with evaluation order as brought up wrt ponylang/rfcs#182

class Foo
  fun val a_method(other: Foo ref): Bool =>
     this == other
     
class Bar
  var foo: Foo iso
  fun ref getAsVal(): Foo val =>
     foo = Foo

actor Main new create(env: Env) =>
  let bar = Bar
  bar.getAsVal().a_method(bar.foo)

Under Pony's arguments-first evaluation order, we first grab a temporary iso alias to bar.foo, then getAsVal() gives us a val alias which aliases, so we can do any amount of "bad things" by passing sendables (e.g. sending the val to another actor and then modifying)

[jemc]

Thanks for the great succinct example.

Given that this would be unsafe to allow under current Pony semantics, I'll close this issue, and any further discussion on related topics, new semantics, and how to prove safety of things like this can take place in RFCs.

[jemc]

@jasoncarr0 - Thinking about this more, maybe we can trivially allow this as long as the receiver is a local variable (rather than a more complex expression)? That would force the user to evaluate a complex receiver expression first (by assigning it to a local variable), avoiding the issue with order of evaluation and still allowing a lot of useful cases.

[jasoncarr0]

That possibility will work, but is weaker. It definitely works because if it's a variable it's iso/trn, and so disjoint from any other path of relevance

[jasoncarr0]

Also: amusing timing here, as I just updated the RFC to fix/simplify the conditions a bit.

[jasoncarr0]

Thinking about this, with that condition above, in fact, it's almost exactly the same as recover blocks (which can be fine, for convenience). Since if you've bound it, you can put it in a var by definition:

var x = ... // meet the condition
x = recover
   let x_ref: ... = consume x
   foo(x_ref, ...)
   x_ref
end

[jasoncarr0]

As to the question from sync about which examples don't work with this, I would still point to array as being a particularly common case, or any field. Under these rules, like with recover blocks, we have to remove the object from the array before we can use it here, so this helps a few cases, but it's peeling away only one layer of shuffling.

This gets particularly bad if we've received an array of iso and we don't have any "fake item" to put so to speak. I think I could see the argument that in Pony today, you need a mutable field for iso to be useful. Under updated viewpoint adaptation we can also consume the source before projecting a field to get an ephemeral

[jemc]

I don't understand what you mean - can you post a full example?

Today, I can use auto-recovery to call methods of an iso field without juggling in a new object, for example:

class Object
  var field: String iso

  new create(field': String iso) =>
    field = consume field'

actor Main
  new create(env: Env) =>
    let obj = Object.create(recover String.>push('H').>push('i') end)
    obj.field.push('!')
    // obj.field now says "Hi!"

Now, with the improvement discussed in this ticket, I could also define a method that operates on String ref as an argument, which would be allowed to do arbitrarily complex ref and box operations on the field (like iterating over it, for example) without having to juggle a new value in.

class Object
  var field: String iso

  new create(field': String iso) =>
    field = consume field'

primitive StringDoubler
  fun apply(string: String ref) =>
    let copy = String
    for byte in string.values() do
      copy.push(byte)
    end
    for byte in copy.values() do
      string.push(byte)
    end

actor Main
  new create(env: Env) =>
    let obj = Object.create(recover String.>push('H').>push('i') end)
    StringDoubler(obj.field)
    // obj.field now says "HiHi"

[jasoncarr0]

Sorry, in your case above, automatic recovery would be banned by the requirement that the expression be a simple variable. I did not interpret it to mean fields as fields can be invalidated by other expressions.

I'd not like to expand on the examples here as they're already split between too many places, so we should take this to Zulip or another medium in that case.

[jemc]

The "simple variable" requirement only applies to the receiver in the rules we discussed above, not the argument.

It's true that my example did not use a literal variable as the receiver (it used the primitive name StringDoubler), but for the purposes of preventing evaluation order issues in the rule set we can probably consider that to be just as "simple" as a variable.

[jasoncarr0]

Ah, this was a miscommunication in that case. Yeah, that case is not a priori sound because other arguments could invalidate. x.foo(y.x, y.method_invalidating_x()) this will evaluate the method after evaluating y.x

[jemc]

So both the receiver and all arguments that follow the auto-recovered argument need to be a "simple" expression requiring no evaluation, but it seems that the auto-recovered argument itself can be.

So my example with StringDoubler(obj.field) should still be valid, right?

And other cases can be made valid just by making sure that the arguments are in an order such that the auto-recovered one is either last in the list, or by making sure to use simple variables for those arguments that follow it.

[jasoncarr0]

Yeah, that would suffice. So long as anything evaluated after the target of the recover cannot invalidate (such as being just a simple variable/consume; a constructor with no arguments, or read-only), then it works. Your example should work because StringDoubler.create() mentions no writeable aliases which could alias obj.