On Fri, Mar 15, 2024, at 11:25 PM, Rowan Tommins [IMSoP] wrote:
> On 15 March 2024 17:11:29 GMT, Larry Garfield <la...@garfieldtech.com> wrote:
>>On Wed, Mar 13, 2024, at 10:26 PM, Rowan Tommins [IMSoP] wrote:
>>> I think it would be more helpful to justify this design on its own 
>>> merits, particularly because it's a significant difference from other 
>>> languages (which either don't have a "real property" behind the hooks, 
>>> or in Kotlin's case allow access to it only *directly* inside the hook 
>>> definitions, via the "field" keyword).
>>
>>I'm not sure I follow.  The behavior we have currently is very close to how 
>>Kotlin works, from a user perspective.
>
>
> Unless I'm misunderstanding something, the backing field in Kotlin is 
> accessible only inside the hooks, nowhere else. I don't know what would 
> happen if a hook caused a recursive call to itself, but there's no 
> mention in the docs of it bypassing the hooks, only this:
>
>> This backing field can be referenced in the accessors using the `field` 
>> identifier
>
> and
>
>> The `field` identifier can only be used in the accessors of the property.
>
> And then a section explaining that more complex hooks should use a 
> separate backing property - which is the only option in C#, and roughly 
> what people would do in PHP today with __get and __set.
>
> Kotlin does have a special syntax for "delegating" hooks, but looking 
> at the examples, they do not use the backing field at all, they have to 
> provide their own storage.
>
>
>
>>I've lost track of which specific issue you have an issue with or would want 
>>changed.  The guards to prevent an infinite loop are necessary, for the same 
>>reasons as they are necessary for __get/__set.
>
> I understand that *something* needs to happen if a recursive call 
> happens, but it could just be an error, like any other unbounded 
> recursion. 
>
> I can also understand the temptation to make it something more useful 
> than an error, and provide a way to access the "backing field" / "raw 
> value" from outside the hook. But it does lead to something quite 
> surprising: the same line of code does different things depending on 
> how it is called.
>
> I doubt many people have ever discovered that __get and __set work that 
> way, since as far as I can see it's only possible to use deliberately 
> if you're dynamically adding and unsetting properties inside your class.
>
> So, I don't necessarily think hooks working that way is the wrong 
> decision, I just think it's a decision we should make consciously, not 
> one that's obvious.

Well, reading/writing from within a set/get hook is an obvious use case to 
support.  We cannot do cached properties easily otherwise:

public string $expensive {
  get => $this->expensive ??= $this->compute();
  set { 
    if (strlen($value) < 50) throw new Exception();
    $this->expensive = $value;
  }
}

So disabling the hooks from within the hooks seems like the only logical 
solution there.  (Short of bringing back $field and making it mandatory, which 
is actually much harder than it sounds because of the ++ et al operators that 
would need to be supported.)

The other case then becomes:

class Foo {
  public string $a {
    get => $this->expensive ??= $this->compute();
    set { 
      if (strlen($value) < 50) throw new Exception();
      $this->expensive = $value;
    }
  }

  public function compute() {
    $start = $this->expensive ?? 'a';
     return $start . 'b';
  }
}

Inside compute(), the logic requires reading from $this->expensive.  If we have 
no guard, that would cause an infinite loop.  If the guard extends down the 
call stack, then the loop is eliminated.

That does mean, as you note, that if you call $foo->compute(), the first call 
to $this->expensive will invoke the get hook, but subsequent calls to it will 
not.  That may seem odd, but in practice, we do not see any other alternative 
that doesn't make infinite loops very easy to write.  This is, of course, a 
highly contrived example.  In practice, I don't expect it to come up much in 
the first place.

It was this logic that led us to the current implementation: Once you're within 
the callstack of a hook, you bypass that property's hooks.  We then noted that 
__get/__set have essentially the same guard logic.  It doesn't come up often 
because, again as you note, it's only relevant in even more contrived examples.

So yes, the current logic is very deliberate and based on a process of 
elimination to arrive at what we feel is the only logical design.  Likely the 
same process of elimination that led to the behavior of the guards on 
__get/__set, which is why they are essentially the same.  Being the same also 
makes the language more predictable, which is also a design goal for this RFC.  
(Hence why "this is the same logic as methods/__get/other very similar thing" 
is mentioned several times in the RFC.  Consistency in expectations is 
generally a good thing.)

In theory we could also forbid accessing a property within the call stack of 
its hooks; in that case, the above code would simply error.  However, that is a 
less-optimal solution because then compute() is only sometimes callable, 
depending on the callstack.  That is no less weird than $this->expensive 
skipping hooks only sometimes.  It would also be inconsistent with how 
__get/__set work, which makes hooks less capable and less consistent.  That's 
why we don't think that's a good way of doing it.

Also, we've rewritten the references and arrays sections in the RFC.  Nothing 
actually changed in the implementation, but it should be a lot clearer now that 
only a fairly small subset of impossible situations are blocked.  Most things 
"just work," and when they don't, they (again) don't for a logical reason that 
parallels the behavior of the language elsewhere.  There's even some nice 
summary tables. :-)

--Larry Garfield

Reply via email to