Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122650
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-300-gdee1775a43-fm-20240315.001-gdee1775a
Precedence: bulk
MIME-Version: 1.0
Message-ID: <e18863f1-9cc9-400b-b6b4-d1be62fa4d0f@app.fastmail.com>
In-Reply-To: <C517590C-77D1-462C-92E9-D2851345BE67@rwec.co.uk>
References: <fca97d6c-d4ec-4000-958a-72c32beb8421@app.fastmail.com>
 <a715dacb-9bac-4bea-a2bb-c623737fd81e@app.fastmail.com>
 <c057b162-e2fe-4ade-b27f-e09815743748@rwec.co.uk>
 <7eada0fd-39c5-4a89-8c74-80c671801a2d@app.fastmail.com>
 <1698692e-8eb1-4bfc-a743-375696cd8f1c@rwec.co.uk>
 <154481e0-5f62-4026-994a-28a644d71527@app.fastmail.com>
 <C517590C-77D1-462C-92E9-D2851345BE67@rwec.co.uk>
Date: Sat, 16 Mar 2024 00:19:57 +0000
To: "php internals" <internals@lists.php.net>
Subject: Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
Content-Type: text/plain
From: larry@garfieldtech.com ("Larry Garfield")

On Fri, Mar 15, 2024, at 11:25 PM, Rowan Tommins [IMSoP] wrote:
> On 15 March 2024 17:11:29 GMT, Larry Garfield <larry@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