Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118563
MIME-Version: 1.0
References: <CAOWwgpk7uAkZD_CH5inbm8Ng8iVL5SZsz59Mpzgpc_iG42ei8Q@mail.gmail.com>
In-Reply-To: <CAOWwgpk7uAkZD_CH5inbm8Ng8iVL5SZsz59Mpzgpc_iG42ei8Q@mail.gmail.com>
Date: Mon, 5 Sep 2022 11:01:00 +0200
Message-ID: <CAH5C8xU=eukkqn+PToG4-JT0vqjChhkY=kd_4t_ki9mXu1UMqQ@mail.gmail.com>
To: Nicolas Grekas <nicolas.grekas+php@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000003d8d0005e7ea52af"
Subject: Re: Issues with readonly classes
From: kocsismate90@gmail.com (=?UTF-8?B?TcOhdMOpIEtvY3Npcw==?=)

--0000000000003d8d0005e7ea52af
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Nicolas,

For 2.: static properties are useful e.g. to cache some shared state (info
> extracted from reflection in my case) and I really don't see why readonly
> classes should forbid static properties. Readonly is only useful for
> instances, because it gives them immutability, but what's the link with
> static properties? Note that this is less important than the previous poi=
nt
> as it's always possible to store shared global state in another class. Bu=
t
> still, this feels arbitrary to me.
>

First, I'll answer to your question about static properties, because the
answer is simple: as readonly static properties are not supported (
https://wiki.php.net/rfc/readonly_properties_v2#restrictions), I had to
decide what to do with static properties of readonly classes (
https://github.com/php/php-src/pull/7305#issuecomment-971381308), so Nikita
and I agreed that forbidding them is the better option until we add support
for static readonly properties. In my opinion, being able to set readwrite
static properties for readonly classes would be very counterintuitive.

But I'm also wondering about B: the RFC doesn't contain any rationale about
> why this restriction exists (it says "Similarly how overriding of readonl=
y
> properties works", but the similarity is quite weak, readonly props don't
> apply to other properties of child classes.) If there is no compelling
> reason to have this limitation, it should be removed IMHO. Basically, thi=
s
> would mean that child classes of readonly classes wouldn't have to be
> readonly themselves (but the existing properties on the parent would have
> to be of course.)


Regarding your main question: I understand your problem with readonly
classes, and I'd be happy if we found a solution which fits your use-cases
and keeps consistency for the engine at the same time. To give you more
context about the inheritance related restriction in the RFC: I went with
forbidding the extension of readonly classes by non-readonly ones so that
the invariants of the parent (no dynamic or mutable properties are allowed)
don't occasionally get violated by the child. However, I cannot think about
a proper scenario now where this would cause any problems... I'm wondering
if anyone could come up with one?

And at last, regarding the interaction of readonly properties with
__clone(): I'll definitely think more about this topic in the following
weeks. Currently, it seems to me that readonly properties should be able to
be reinitialized once *during* cloning, but not afterwards. Is this in line
with what you want to achieve? If we once manage to add support for the
"clone with" construct then readonly property reassignment(s) due to "with"
arguments should also be allowed. But I'll need more time to also consider
any undesirable side effects/edge cases, which could apply, should we
settle on the approach described above.

Regards,
M=C3=A1t=C3=A9

--0000000000003d8d0005e7ea52af--