Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118554
MIME-Version: 1.0
Date: Fri, 2 Sep 2022 19:30:49 +0200
Message-ID: <CAOWwgpk7uAkZD_CH5inbm8Ng8iVL5SZsz59Mpzgpc_iG42ei8Q@mail.gmail.com>
To: PHP Internals List <internals@lists.php.net>, =?UTF-8?B?TcOhdMOpIEtvY3Npcw==?= <kocsismate90@gmail.com>
Content-Type: multipart/alternative; boundary="000000000000efda1605e7b5172a"
Subject: Issues with readonly classes
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--000000000000efda1605e7b5172a
Content-Type: text/plain; charset="UTF-8"

Hello everybody, hi Mate

I played with readonly classes recently, to add support for them to
lazy-loading proxy generation and I struggled upon what I see as serious
issues.

As noted in the RFC, and because the readonly flag must be set on child
classes, those cannot:

   1. Declare a new non-readonly property;
   2. Have static properties;
   3. Have dynamic properties.

I figured out 1. and 2. not by reading the RFC but because I needed those
capabilities in the generated inheritance proxies.

1. is the most serious one: being able to declare a non-readonly property
is the only way to implement a cloneable proxy (to be allowed to do
$this->realInstance = clone $this->realInstance; in __clone()). I think
there are two ways to solve the use case I have: A. fix the
non-cloneability of readonly props or B. allow child classes to add
non-readonly properties. Because to me this is a serious restriction, A or
B should ideally be in 8.2. I think A is a must-have so that might be the
thing to do. 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 readonly 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, this 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.)

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 point
as it's always possible to store shared global state in another class. But
still, this feels arbitrary to me.

For 3. I just noted the limitation to be exhaustive, but I'm really fine if
dynamic properties remain forbidden on child classes. It might be seen as
inconsistent with other classes, but dynamic properties don't have any
purpose anyway since there is always an alternative to them.

I understand there are good intentions behind these limitations (making PHP
a stricter language). But here, I feel like this is detrimental to the
language, by making it a minefield of kinda arbitrary dos and don'ts. At
least that's what I feel here.

Does that make sense to you? Should we remove the limitations I mention?

Nicolas
PS: About allowing readonly props to be reinitialized during cloning, I
tried https://github.com/php/php-src/pull/9403 but I failed as I can't make
it work. I'd appreciate any help on the topic. I'd even be happy to close
if someone else would like to work on the topic. Could that be you?

--000000000000efda1605e7b5172a--