Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122692
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: <452aae1b-8929-4f08-a388-45b374d2a365@app.fastmail.com>
In-Reply-To: <ce92f75c-1a59-a469-2a04-6ff391eb2361@php.net>
References: <fca97d6c-d4ec-4000-958a-72c32beb8421@app.fastmail.com>
 <a715dacb-9bac-4bea-a2bb-c623737fd81e@app.fastmail.com>
 <ce92f75c-1a59-a469-2a04-6ff391eb2361@php.net>
Date: Tue, 19 Mar 2024 17:26:25 +0000
To: "php internals" <internals@lists.php.net>
Subject: Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
From: larry@garfieldtech.com ("Larry Garfield")

On Tue, Mar 19, 2024, at 11:16 AM, Derick Rethans wrote:
> On Fri, 8 Mar 2024, Larry Garfield wrote:
>
>> Hi folks.  Based on earlier discussions, we've made a number of=20
>> changes to the RFC that should address some of the concerns people=20
>> raised.  We also had some very fruitful discussions off-list with=20
>> several developers from the Foundation, which led to what we feel are=20
>> some solid improvements.
>>=20
>> https://wiki.php.net/rfc/property-hooks
>
> Some comments and questions:
>
> 	Be aware, the detection logic works on $this->[propertyName] directly=
 at=20
> 	compile time, not on dynamic forms of it like $prop =3D 'beep';=20
> 	$this->$prop. That will not trigger a backing value.=20
>
> How can that not cause issues?

For most uses that should be fine.  But it's more that trying to do anyt=
hing else would be vastly more complicated and error prone for the imple=
mentation.  And most people hated the alternate $field variable from Kot=
lin.

> 	 The set hook's return type is unspecified, and will silently be=20
> 	treated as void.=20
>
> What happens if you *do* specify a return type? Will it Error?

Yes, it's a parse error.

> 	Implicit ''set'' parameter
>
> 	If the write-type of a property is the same as its defined type=20
> 	(this is the common case), then the argument may be omitted=20
> 	entirely.=20
>
> 	=E2=80=A6
>
> 	If the parameter is not specified, it defaults to $value.
>
> I am not a fan of this "magical" behaviour. Do we *need* this short cu=
t,=20
> and the following "Short-set"?

We believe we do.  One of the goals of the RFC is to reduce and avoid ve=
rbose boilerplate.  The short-hands should be the common case, in practi=
ce.  The only reason one would have to specify a set parameter is if the=
re was a good reason to change the variable name or widen the set type. =
 Both of those are a-typical situations.

> 	With asymmetric visibility that was previously proposed, the=20
> 	example can be further simplified.=20
>
> But it isn't here, so why is this example (and the next one) in the RF=
C?=20
> :-)

Many/most languages with accessors also have asymmetric visibility.  Nik=
ita's original RFC also combined them all into one.  The two RFCs are no=
t mutually dependent, but are mutually-supportive, by design.  So this c=
omes under the "why didn't we include feature X" heading, with an answer=
 "that's in a separate RFC."

> 	Interaction with constructor property promotion
>
> 	=E2=80=A6 In particular, the shorthand version of hook bodies and the=20
> 	ability to call out to private methods if they get complicated=20
> 	partially obviate the concern about syntactic complexity.=20
>
> Although that is true, it *does* add more complexity in tools that nee=
ds=20
> to parse PHP, as there is now another piece of new syntax that needs t=
o=20
> be added (and tested with).

That's true, but it's also inherently true of hooks themselves.  And any=
 other syntax improvement to the language.

> 	ReflectionProperty has several new methods to work with hooks.
>
> 	getHooks(): array returns an array of \ReflectionMethod objects =20
> 	keyed by the hook they are for.
>
> What will the name for the &get hook be? And shouldn't there be an enu=
m=20
> case for that as well?

&get isn't its own hook.  It's still a "get" hook, it just returns by re=
ference.  So it will be available with the "Get" enum value.  If the ret=
urn-by-ref status needs to be known for whatever reason, that is already=
 available on the ReflectionMethod object that is returned.

--Larry Garfield