Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122680
Feedback-ID: id5114917:Fastmail
Content-Type: multipart/alternative;
 boundary="------------xsd2ZODV07fgca0wbANEqk14"
Message-ID: <771e196c-274b-428a-a13c-4078a3f5ecfa@rwec.co.uk>
Date: Mon, 18 Mar 2024 11:36:04 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
To: internals@lists.php.net
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>
 <e18863f1-9cc9-400b-b6b4-d1be62fa4d0f@app.fastmail.com>
 <46609F15-DD40-4BD2-A78A-16021C3447C8@rwec.co.uk>
 <CAPyj-LBEzCNzWkh8qwfBrC-ccMKFANX=zwNq9O0qSnNdFNjtzw@mail.gmail.com>
 <f95527c0-a0d5-48da-a071-ce1c4ffe6814@rwec.co.uk>
 <CAPyj-LC2=pdo79AbfBJbyb-e8JbZyswsaBeuFq=Zv80JpHs3fw@mail.gmail.com>
 <d1ca7dbe-71a3-40fc-a88e-e64d11606ea7@rwec.co.uk>
 <CAPyj-LD7jBpF=3+61hjhTc_TGgUMejVWvWHRDrVxZebGdCBEKw@mail.gmail.com>
Content-Language: en-GB
In-Reply-To: <CAPyj-LD7jBpF=3+61hjhTc_TGgUMejVWvWHRDrVxZebGdCBEKw@mail.gmail.com>
From: imsop.php@rwec.co.uk ("Rowan Tommins [IMSoP]")

This is a multi-part message in MIME format.
--------------xsd2ZODV07fgca0wbANEqk14
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 18/03/2024 00:04, Ilija Tovilo wrote:
> I realize this is somewhat inconsistent, but I believe it is
> reasonable. If you want to expose the underlying property
> by-reference, you need to jump through some additional hoops.


I disagree with this reasoning, because I foresee plenty of cases where 
a virtual property is necessary anyway, so doesn't provide any 
additional hoop to jump through.

But there's not much more to say on this point, so I guess we'll leave 
it there.



> Again, it depends on how you think about it. As you have argued, for a
> get-only property, the backing value should not be writable without an
> explicit `set;` declaration. You can interpret `set;` as an
> auto-generated hook, or as a marker that indicates that the backing
> value is accessible without a hook.


Regardless of which of these views you start with, it still seems 
intuitive to me that accesses inside the get hook would bypass the 
normal rules and write to the raw value.

Leaving aside the implementation, there are three things that can happen 
when you write to a property:

a) the set hook is called
b) the raw property is written to
c) an error is thrown

Inside the dynamic scope of a hook, the behaviour is always (b), and I 
don't see any reason for that to change. From anywhere else, backed 
properties currently try (a) and fall back to (b); virtual properties 
try (a) and fall back to (c).

I do understand that falling back to (b) makes the implementation 
simpler, and works well with inheritance and some use cases; but falling 
back to (c) wouldn't necessarily need a "default hook", just a marker of 
"has hooks".

It occurred to me you could implement it in reverse: auto-generate a 
hook "set => throw new Error;" and then *remove* it if the user opts in 
to the default set behaviour. That would keep the "write directly" case 
optimised "for free"; but it would be awkward for inheritance, as you'd 
have to somehow avoid calling the parent's hook.



> The meaning for `set;` is no longer clear. Does it mean that there's a
> generated hook that accesses the backing field? Does it mean that the
> backing field is accessible without a hook? Or does it mean that it
> accesses the parent hook? The truth is, with inheritance there's no
> way to look at the property declaration and fully understand what's
> going on, unless all hooks must be spelled out for the sake of clarity
> (e.g. `get => parent::$prop::get()`).


Yes, I think this is probably a good argument against requiring "set;"

I think "be careful when inheriting only one hook" will always be a key 
rule to teach anyway, because it's easy to mess up (e.g. assuming the 
parent is backed and accessing $this->foo, rather than calling the 
parent's hook implementation). But adding "set;" into the mix probably 
just makes it worse.



> I seriously doubt accessing the backing value outside of the current
> hook is useful. The backing value is an implementation detail. If it
> is absolutely needed, `ReflectionProperty::setRawValue()` offers a way
> to do it. I understand the desire for a shorter alternative like
> `$field`, but it doesn't seem like the majority shares this desire at
> this point in time.


The example of clearAll() is a real use case, which people will 
currently achieve with __get and __set (e.g. the Yii ActiveRecord 
implementation I linked in one of my previous messages).

The alternative wouldn't be reflection, it would just be switching to a 
virtual property with the value stored in a private field. I think 
that's fine, it's just drawing the line of which use cases backed 
properties cover: Kotlin covers more use cases than C#; PHP will cover 
more than Kotlin (methods able to by-pass a hook when called from that 
hook); but it will draw the line here.



> A different syntax like `$this->prop::raw` comes with similar
> complexity issues, similar to those previously discussed for
> `parent::$prop`/`parent::$prop = 'prop'`.


Yeah, I can't even think of a nice syntax for it, let alone a nice 
implementation. Let's leave it as a thought experiment, no further 
action needed. :)


Regarding asymmetric types:

> I can't speak for IDEs or static
> analyzers, but I'm not sure what makes this case special. We can ask
> some of their maintainers for feedback.


In order to reliably tell the user whether "$a->foo = $b->bar;" is a 
type-safe operation, the analyser will need to track two types for every 
property, the "gettable type" and the "settable type", and apply them in 
the correct contexts.

I've honestly no idea whether that will be easy or hard; it will 
probably vary between tools. In particular, I get the impression IDEs / 
editor plugins sometimes have a base implementation used for multiple 
programming languages, and PHP might be the only one that needed this 
extra tracking.


Regards,

-- 
Rowan Tommins
[IMSoP]

--------------xsd2ZODV07fgca0wbANEqk14
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">On 18/03/2024 00:04, Ilija Tovilo
      wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAPyj-LD7jBpF=3+61hjhTc_TGgUMejVWvWHRDrVxZebGdCBEKw@mail.gmail.com"><span
      style="white-space: pre-wrap">
</span>
      <pre class="moz-quote-pre" wrap="">I realize this is somewhat inconsistent, but I believe it is
reasonable. If you want to expose the underlying property
by-reference, you need to jump through some additional hoops.</pre>
    </blockquote>
    <p><br>
    </p>
    <p>I disagree with this reasoning, because I foresee plenty of cases
      where a virtual property is necessary anyway, so doesn't provide
      any additional hoop to jump through.</p>
    <p>But there's not much more to say on this point, so I guess we'll
      leave it there.<br>
    </p>
    <p><br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite"
cite="mid:CAPyj-LD7jBpF=3+61hjhTc_TGgUMejVWvWHRDrVxZebGdCBEKw@mail.gmail.com"><span
      style="white-space: pre-wrap">
</span>
      <pre class="moz-quote-pre" wrap="">Again, it depends on how you think about it. As you have argued, for a
get-only property, the backing value should not be writable without an
explicit `set;` declaration. You can interpret `set;` as an
auto-generated hook, or as a marker that indicates that the backing
value is accessible without a hook.</pre>
    </blockquote>
    <p><br>
    </p>
    <p>Regardless of which of these views you start with, it still seems
      intuitive to me that accesses inside the get hook would bypass the
      normal rules and write to the raw value.</p>
    <p>Leaving aside the implementation, there are three things that can
      happen when you write to a property:<br>
    </p>
    <p>a) the set hook is called<br>
      b) the raw property is written to<br>
      c) an error is thrown</p>
    <p>Inside the dynamic scope of a hook, the behaviour is always (b),
      and I don't see any reason for that to change. From anywhere else,
      backed properties currently try (a) and fall back to (b); virtual
      properties try (a) and fall back to (c).<br>
    </p>
    I do understand that falling back to (b) makes the implementation
    simpler, and works well with inheritance and some use cases; but
    falling back to (c) wouldn't necessarily need a "default hook", just
    a marker of "has hooks".<br>
    <p>It occurred to me you could implement it in reverse:
      auto-generate a hook "set =&gt; throw new Error;" and then
      *remove* it if the user opts in to the default set behaviour. That
      would keep the "write directly" case optimised "for free"; but it
      would be awkward for inheritance, as you'd have to somehow avoid
      calling the parent's hook.<br>
    </p>
    <p><br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite"
cite="mid:CAPyj-LD7jBpF=3+61hjhTc_TGgUMejVWvWHRDrVxZebGdCBEKw@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">The meaning for `set;` is no longer clear. Does it mean that there's a
generated hook that accesses the backing field? Does it mean that the
backing field is accessible without a hook? Or does it mean that it
accesses the parent hook? The truth is, with inheritance there's no
way to look at the property declaration and fully understand what's
going on, unless all hooks must be spelled out for the sake of clarity
(e.g. `get =&gt; parent::$prop::get()`).
</pre>
    </blockquote>
    <p><br>
    </p>
    <p>Yes, I think this is probably a good argument against requiring
      "set;" <br>
    </p>
    <p>I think "be careful when inheriting only one hook" will always be
      a key rule to teach anyway, because it's easy to mess up (e.g.
      assuming the parent is backed and accessing $this-&gt;foo, rather
      than calling the parent's hook implementation). But adding "set;"
      into the mix probably just makes it worse.</p>
    <br>
    <p><br>
    </p>
    <span style="white-space: pre-wrap">
</span><span style="white-space: pre-wrap">
</span>
    <blockquote type="cite"
cite="mid:CAPyj-LD7jBpF=3+61hjhTc_TGgUMejVWvWHRDrVxZebGdCBEKw@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">I seriously doubt accessing the backing value outside of the current
hook is useful. The backing value is an implementation detail. If it
is absolutely needed, `ReflectionProperty::setRawValue()` offers a way
to do it. I understand the desire for a shorter alternative like
`$field`, but it doesn't seem like the majority shares this desire at
this point in time.</pre>
    </blockquote>
    <p><br>
    </p>
    <p>The example of clearAll() is a real use case, which people will
      currently achieve with __get and __set (e.g. the Yii ActiveRecord
      implementation I linked in one of my previous messages). </p>
    <p>The alternative wouldn't be reflection, it would just be
      switching to a virtual property with the value stored in a private
      field. I think that's fine, it's just drawing the line of which
      use cases backed properties cover: Kotlin covers more use cases
      than C#; PHP will cover more than Kotlin (methods able to by-pass
      a hook when called from that hook); but it will draw the line
      here.<br>
    </p>
    <p><br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite"
cite="mid:CAPyj-LD7jBpF=3+61hjhTc_TGgUMejVWvWHRDrVxZebGdCBEKw@mail.gmail.com"><span
      style="white-space: pre-wrap">
</span>
      <pre class="moz-quote-pre" wrap="">A different syntax like `$this-&gt;prop::raw` comes with similar
complexity issues, similar to those previously discussed for
`parent::$prop`/`parent::$prop = 'prop'`.</pre>
    </blockquote>
    <p><br>
    </p>
    <p>Yeah, I can't even think of a nice syntax for it, let alone a
      nice implementation. Let's leave it as a thought experiment, no
      further action needed. :)<br>
    </p>
    <p><br>
    </p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <p><span style="white-space: pre-wrap">Regarding asymmetric types:
</span></p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <blockquote type="cite"
cite="mid:CAPyj-LD7jBpF=3+61hjhTc_TGgUMejVWvWHRDrVxZebGdCBEKw@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">I can't speak for IDEs or static
analyzers, but I'm not sure what makes this case special. We can ask
some of their maintainers for feedback.</pre>
    </blockquote>
    <p><br>
    </p>
    <p>In order to reliably tell the user whether "$a-&gt;foo =
      $b-&gt;bar;" is a type-safe operation, the analyser will need to
      track two types for every property, the "gettable type" and the
      "settable type", and apply them in the correct contexts.</p>
    <p>I've honestly no idea whether that will be easy or hard; it will
      probably vary between tools. In particular, I get the impression
      IDEs / editor plugins sometimes have a base implementation used
      for multiple programming languages, and PHP might be the only one
      that needed this extra tracking.<br>
    </p>
    <p><br>
    </p>
    <span style="white-space: pre-wrap">
</span>
    <p>Regards,<br>
    </p>
    <pre class="moz-signature" cols="72">-- 
Rowan Tommins
[IMSoP]</pre>
  </body>
</html>

--------------xsd2ZODV07fgca0wbANEqk14--