Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122667
Feedback-ID: id5114917:Fastmail
Message-ID: <d1ca7dbe-71a3-40fc-a88e-e64d11606ea7@rwec.co.uk>
Date: Sun, 17 Mar 2024 14:41:19 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
Content-Language: en-GB
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>
In-Reply-To: <CAPyj-LC2=pdo79AbfBJbyb-e8JbZyswsaBeuFq=Zv80JpHs3fw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
From: imsop.php@rwec.co.uk ("Rowan Tommins [IMSoP]")

On 17/03/2024 00:01, Ilija Tovilo wrote:
> For clarity, you are asking for a way to make the "virtualness" of
> properties more explicit, correct?


Either more explicit, or less important: the less often the user needs 
to know whether a property is virtual, the less it matters how easily 
they can find out.



> Please let me know if
> you are aware of any other potentially non-intuitive cases.


I agree that while they may not be immediately obvious to the user, most 
of the distinctions do make sense once you think about them.

The remaining difference I can see in the current RFC which seems to be 
unnecessary is that combining &get with set is only allowed on virtual 
properties. Although it may be "virtual" in the strict sense, any &get 
hook must actually be referring to some value stored somewhere - that 
might be a backed property, another field on the current class, a 
property of some other object, etc:

public int $foo { &get => $this->foo; set { $this->foo = $value; } }

public int $bar { &get => $this->_bar; set { $this->_bar = $value; } }

public int $baz { &get => $this->delegatedObj->baz; set { 
$this->delegatedObj->baz = $value; } }

This sentence from the RFC applies equally to all three of these examples:

 > That is because any attempted modification of the value by reference 
would bypass a |set| hook, if one is defined.

I suggest that we either trust the user to understand that that will 
happen, and allow combining &get and set on any property; or we do not 
trust them, and forbid it on any property.



> Apart from the things already mentioned, it's unclear to me whether,
> with such `set;` declarations, a `get`-only backed property should
> even be legal. With the complete absence of a write operation, the
> assignment within the `set` itself would fail. To make this work, the
> absence of `set;` would need to mean something like "writable, but
> only within another hook", which introduces yet another form of
> asymmetric visibility.


Any write inside the get hook already by-passes the set hook and refers 
to the underlying property, so there would be no need for any default 
set behaviour other than throwing an error.

It's not likely to be a common scenario, but the below works with the 
current implementation https://3v4l.org/t7qhR/rfc#vrfc.property-hooks

class Example {
     public int $nextNumber {
         get {
             $this->nextNumber ??= 0;
             return $this->nextNumber++;
         }
         // Mimic the current behaviour of a virtual property: 
https://3v4l.org/cAfAI/rfc#vrfc.property-hooks
         set => throw new Error('Property Example::$nextNumber is 
read-only');
     }
}



> Fair enough. 1 and 2 are reasons why we added the `$field` macro as an
> alternative syntax in the original draft. I don't quite understand
> point 3. In Kotlin, `field` is only usable within its associated hook.
> Other languages I'm aware of do not provide a way to access the
> backing value directly, neither inside nor outside the accessor.


We are already allowing more than Kotlin by letting hooks call out to a 
method, and have that method refer back to the raw value. 
Hypothetically, we could allow *any* method to access it, using some 
syntax like $this->foo::raw. As a spectrum from least access to most access:

1) $field - accessible only in the lexical scope of the hook

2) $this->foo - accessible in the dynamic scope of the hook, e.g. a hook 
calling $this->doSomething(__PROPERTY__);

3) $this->foo::raw - accessible anywhere in the class, e.g. a public 
clearAll() method by-passing hooks

Whichever we provide for backed properties, option 3 is available for 
virtual properties anyway, and common with __get/__set: store a value in 
a private property, and have a public hooked property providing access 
to it.

I understand now that option 2 fits most easily with the implementation, 
and with decisions around inheritance and upgrade of existing code; but 
the other options do have their advantages from a user's point of view.



> I personally do not feel strongly about whether asymmetric types make
> it into the initial implementation. Larry does, however, and I think
> it is not fair to exclude them without providing any concrete reasons
> not to. I will spend time in the following days cleaning up tests, and
> I will try my best to try to break asymmetric types. If I (or anybody
> else) can't find a way to do so, I don't see a reason to remove them.


My concern is more about the external impact of what is effectively a 
change to the type system of the language: will IDEs give correct 
feedback to users about which assignments are legal? will tools like 
PhpStan and Psalm require complex changes to analyse code using such 
properties? will we be prevented from adding some optimisation to 
OpCache because these properties break some otherwise safe assumption?

Maybe I'm being over-cautious, but those are the kinds of questions I 
would expect to come up if this feature had its own RFC.

Regards,

-- 
Rowan Tommins
[IMSoP]