Re: [PHP-DEV] Re: Issues with readonly classes
Hi, > If my default assumption (and Larry's) was that such a class would be immutable, it's fair to think that a non-trivial number of other programmers may make the same faulty assumption, and making this distinction obvious in the documentation would probably help. Yes, it makes sense to mention this in the documentation, but fortunately, the manual already makes the behavior clear at https://www.php.net/manual/en/language.oop5.properties.php#language.oop5.properties.readonly-properties : "However, readonly properties do not preclude interior mutability. Objects (or resources) stored in readonly properties may still be modified internally:" Following our discussion, Nicolas and I announced our proposal to fix two of the above mentioned issues: https://externals.io/message/119007 Feedback is welcome! Regards: Máté Jordan LeDoux ezt írta (időpont: 2022. szept. 25., V, 23:01): > > > On Sun, Sep 25, 2022 at 10:57 AM Máté Kocsis > wrote: > >> Hi, >> >> I agree with Tim, and I also think that both reverting and making any last >> minute fundamental change is not a good idea, especially >> because people don't have a clear agreement about how inheritance >> should work. Readonly classes is an optional feature, so anyone >> who wants proxies to work can simply not add the readonly flag to their >> classes. Of course, it's a bit more tricky for library code, >> but package authors should be aware of this gotcha. Having that said, I'll >> try my best to fix the current shortcomings for PHP 7.3. >> >> Regards, >> Máté >> > > I tried hard to make it clear that I don't think this makes it "broken", > it was just a deviation from my expectations and memory, both of which can > obviously be flawed. I was mostly looking for some kind of information > about what the reasoning was for this, given that I obviously (since I > didn't remember it) missed that part of the discussion. The distinction > between "readonly" and "immutable" is subtle, but fair. This distinction > should probably be pointed out quite explicitly in the documentation > though. > > If my default assumption (and Larry's) was that such a class would be > immutable, it's fair to think that a non-trivial number of other > programmers may make the same faulty assumption, and making this > distinction obvious in the documentation would probably help. > > Jordan >
Re: [PHP-DEV] Re: Issues with readonly classes
On Sun, Sep 25, 2022 at 10:57 AM Máté Kocsis wrote: > Hi, > > I agree with Tim, and I also think that both reverting and making any last > minute fundamental change is not a good idea, especially > because people don't have a clear agreement about how inheritance > should work. Readonly classes is an optional feature, so anyone > who wants proxies to work can simply not add the readonly flag to their > classes. Of course, it's a bit more tricky for library code, > but package authors should be aware of this gotcha. Having that said, I'll > try my best to fix the current shortcomings for PHP 7.3. > > Regards, > Máté > I tried hard to make it clear that I don't think this makes it "broken", it was just a deviation from my expectations and memory, both of which can obviously be flawed. I was mostly looking for some kind of information about what the reasoning was for this, given that I obviously (since I didn't remember it) missed that part of the discussion. The distinction between "readonly" and "immutable" is subtle, but fair. This distinction should probably be pointed out quite explicitly in the documentation though. If my default assumption (and Larry's) was that such a class would be immutable, it's fair to think that a non-trivial number of other programmers may make the same faulty assumption, and making this distinction obvious in the documentation would probably help. Jordan
Re: [PHP-DEV] Re: Issues with readonly classes
Hi, I remembered the same thing, and am similarly baffled. How did the RFC pass > if you can do something as simple as `public readonly stdClass $var;`? I > thought I followed the discussion on that RFC, but apparently I missed > something. I would have expected an example like above to block acceptance > of the RFC. To be clear though, I'm mostly confused about what the > convincing argument about this was, or if it was something that everyone > else viewed as an uncontroversial aspect? > The readonly class RFC doesn't really bring anything new to the table, just complements the readonly property RFC. The latter RFC explicitly mentions that interior mutability of properties is not restricted in any way. In my opinion, this is a wise and pragmatic approach, so that use-cases like the body stream of PSR-7 remain possible. Please note that the feature is called "readonly", not "immutable". What's your take about 8.2? As I demonstrated, readonly classes are broken > because of this propagation to child classes. Does that mean we should > remove this constraint from 8.2? What about reverting the feature and > considering it again for 8.3, after fixing it? I agree with Tim, and I also think that both reverting and making any last minute fundamental change is not a good idea, especially because people don't have a clear agreement about how inheritance should work. Readonly classes is an optional feature, so anyone who wants proxies to work can simply not add the readonly flag to their classes. Of course, it's a bit more tricky for library code, but package authors should be aware of this gotcha. Having that said, I'll try my best to fix the current shortcomings for PHP 7.3. broken. see thread I may be biased, but neither I would call the whole feature "broken". It works as intended for many regular use-cases. However, as you highlighted in the thread, there are some use-cases with which the interoperability of readonly classes is suboptimal, to say the least. This is because the RFC was designed with a conservative approach (e.g. static properties and inheritance by non-readonly classes are forbidden). This way, we'll be able to iteratively improve the feature, while we minimize the risk of introducing undesirable behavior which would be difficult to get rid of later. I know that there is a thin line between "conservative" and "incomplete", but I do hope that PHP 7.3 will be the first version where readonly properties become actually useful. Does anyone else recall this? Máté, are Jordan and I just imagining > things? It may be possible that the discussion extended to this question, but neither me (author of the previous readonly properties RFC), nor Nikita had intention to restrict the usage this way due to the reasons detailed in the first paragraph. Regards, Máté
Re: [PHP-DEV] Re: Issues with readonly classes
On Tue, Sep 20, 2022, at 1:07 PM, Jordan LeDoux wrote: > On Sun, Sep 11, 2022 at 8:22 AM Larry Garfield > wrote: > >> >> >> Hm. I seem to recall during the discussion of readonly classes someone >> saying that object properties of a readonly class had to also be readonly >> classes, which would render the above code a compile error. However, I >> just checked and that is not in the RFC. Was it removed? Am I imagining >> things? Anyone else know what I'm talking about? :-) >> >> --Larry Garfield >> >> > I remembered the same thing, and am similarly baffled. How did the RFC pass > if you can do something as simple as `public readonly stdClass $var;`? I > thought I followed the discussion on that RFC, but apparently I missed > something. I would have expected an example like above to block acceptance > of the RFC. To be clear though, I'm mostly confused about what the > convincing argument about this was, or if it was something that everyone > else viewed as an uncontroversial aspect? > > Jordan Does anyone else recall this? Máté, are Jordan and I just imagining things? --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: Issues with readonly classes
Hi On 9/21/22 11:48, Nicolas Grekas wrote: What's your take about 8.2? As I demonstrated, readonly classes are broken because of this propagation to child classes. s/broken/working as expected broken. see thread Working as expected (or: working as designed). The behavior with regard to inheritance was an explicit section (with its own headline) in the RFC and thus was voted and agreed-on. Changing that would be a non-trivial change from the agreed-on behavior and thus warrants another vote at the very least. The same is true for reverting the readonly class feature entirely, especially since PHP 8.2 is in the RC phase where it's not entirely unreasonable for users to start building on the anticipated features. FWIW personally I would've preferred avoiding this problem by disallowing readonly classes to appear within inheritance hierarchies entirely. Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: Issues with readonly classes
> > > When I was working on the readonly class RFC, I realized that the >> readonly >> > keyword very naturally fits services besides value objects. So my >> > expectation has been that until we can fix the issue with cloning, >> people >> > would mainly apply readonly to services. Not that it is very useful, >> but I >> > would also feel some kind of weird fulfillment by doing so. >> > >> > Regarding cloning: I created a WIP PR not long ago to fix the >> > aforementioned cloning issue, and I'll pursue a readonly amendment RFC >> in >> > the coming weeks (or month) containing the long awaited improvements for >> > cloning (hopefully together with the "clone with" construct) and >> possibly >> > with this inheritance-related change Nicolas proposed, unless someone >> can >> > come up with an ultimate counter-argument. >> > >> >> What's your take about 8.2? As I demonstrated, readonly classes are broken >> because of this propagation to child classes. >> > > s/broken/working as expected > broken. see thread >
Re: [PHP-DEV] Re: Issues with readonly classes
On Wed, 21 Sept 2022 at 11:30, Nicolas Grekas wrote: > Hi all, > > > > When I was working on the readonly class RFC, I realized that the > readonly > > keyword very naturally fits services besides value objects. So my > > expectation has been that until we can fix the issue with cloning, people > > would mainly apply readonly to services. Not that it is very useful, but > I > > would also feel some kind of weird fulfillment by doing so. > > > > Regarding cloning: I created a WIP PR not long ago to fix the > > aforementioned cloning issue, and I'll pursue a readonly amendment RFC in > > the coming weeks (or month) containing the long awaited improvements for > > cloning (hopefully together with the "clone with" construct) and possibly > > with this inheritance-related change Nicolas proposed, unless someone can > > come up with an ultimate counter-argument. > > > > What's your take about 8.2? As I demonstrated, readonly classes are broken > because of this propagation to child classes. > s/broken/working as expected Marco Pivetta https://twitter.com/Ocramius https://ocramius.github.io/
Re: [PHP-DEV] Re: Issues with readonly classes
Hi all, > When I was working on the readonly class RFC, I realized that the readonly > keyword very naturally fits services besides value objects. So my > expectation has been that until we can fix the issue with cloning, people > would mainly apply readonly to services. Not that it is very useful, but I > would also feel some kind of weird fulfillment by doing so. > > Regarding cloning: I created a WIP PR not long ago to fix the > aforementioned cloning issue, and I'll pursue a readonly amendment RFC in > the coming weeks (or month) containing the long awaited improvements for > cloning (hopefully together with the "clone with" construct) and possibly > with this inheritance-related change Nicolas proposed, unless someone can > come up with an ultimate counter-argument. > What's your take about 8.2? As I demonstrated, readonly classes are broken because of this propagation to child classes. Does that mean we should remove this constraint from 8.2? What about reverting the feature and considering it again for 8.3, after fixing it? I'm looking forward to your work on cloning! Note that "clone-with" or similar is a separate issue from the current inability to clone readonly properties within the __clone method. It might be easier to tackle them independently, from a discussion pov. Cheers, Nicolas
Re: [PHP-DEV] Re: Issues with readonly classes
On Sun, Sep 11, 2022 at 8:22 AM Larry Garfield wrote: > > > Hm. I seem to recall during the discussion of readonly classes someone > saying that object properties of a readonly class had to also be readonly > classes, which would render the above code a compile error. However, I > just checked and that is not in the RFC. Was it removed? Am I imagining > things? Anyone else know what I'm talking about? :-) > > --Larry Garfield > > I remembered the same thing, and am similarly baffled. How did the RFC pass if you can do something as simple as `public readonly stdClass $var;`? I thought I followed the discussion on that RFC, but apparently I missed something. I would have expected an example like above to block acceptance of the RFC. To be clear though, I'm mostly confused about what the convincing argument about this was, or if it was something that everyone else viewed as an uncontroversial aspect? Jordan
Re: [PHP-DEV] Re: Issues with readonly classes
Hi Nicolas, Larry, Mate, WDYT? > When I was working on the readonly class RFC, I realized that the readonly keyword very naturally fits services besides value objects. So my expectation has been that until we can fix the issue with cloning, people would mainly apply readonly to services. Not that it is very useful, but I would also feel some kind of weird fulfillment by doing so. Regarding cloning: I created a WIP PR not long ago to fix the aforementioned cloning issue, and I'll pursue a readonly amendment RFC in the coming weeks (or month) containing the long awaited improvements for cloning (hopefully together with the "clone with" construct) and possibly with this inheritance-related change Nicolas proposed, unless someone can come up with an ultimate counter-argument. Regards: Máté
Re: [PHP-DEV] Re: Issues with readonly classes
> >> Personally I'm undecided at the moment whether or not it should be > >> allowed. I'm sympathetic to the "it's easier to disallow and allow > later > >> than vice versa" argument, but still not sure where I stand. The above > at > >> least gives a concrete example of where the problem would be. > >> > > > > If we want the safety you describe, we might want a stronger version of > it. > > Let's name it immutable. An immutable property/class would be like a > > readonly property with the additional restriction that only an immutable > > value could be assigned to it (scalars + array + immutable classes.) But > > that's another topic. > > > > Your example made me doubt for a moment, but without any convincing > > purpose, this should help decide that child classes shouldn't have to be > > readonly. > > > > Nicolas > > Another question I think is pertinent to ask: What kinds of objects > generally would be proxied in this way, and are those the kinds of objects > where a readonly object would make sense? > > Off hand, I'd expect this kind of proxying to be used mainly on services, > whereas readonly classes would be mostly value objects. So the problem > space may be smaller than it initially appears. > It's very difficult to anticipate this. Here is a counter argument: About services: the best ones are of the functional sort; they don't hold any states and their dependencies are also pure functional units. Aka they're immutable and they're good candidates for readonly classes. About entities: e.g. Doctrine proxies them all. So no, I wouldn't say that services are the most common things to proxy, and that entities are the most common things to use readonly on... I hope this discussion illustrated why we should remove propagating readonly to child classes: it serves no purpose and it breaks a universal design pattern. There is no rationale that supports the current behavior. Mate, WDYT? Cheers, Nicolas
Re: [PHP-DEV] Re: Issues with readonly classes
On Sat, Sep 10, 2022, at 4:34 PM, Nicolas Grekas wrote: >> Personally I'm undecided at the moment whether or not it should be >> allowed. I'm sympathetic to the "it's easier to disallow and allow later >> than vice versa" argument, but still not sure where I stand. The above at >> least gives a concrete example of where the problem would be. >> > > If we want the safety you describe, we might want a stronger version of it. > Let's name it immutable. An immutable property/class would be like a > readonly property with the additional restriction that only an immutable > value could be assigned to it (scalars + array + immutable classes.) But > that's another topic. > > Your example made me doubt for a moment, but without any convincing > purpose, this should help decide that child classes shouldn't have to be > readonly. > > Nicolas Another question I think is pertinent to ask: What kinds of objects generally would be proxied in this way, and are those the kinds of objects where a readonly object would make sense? Off hand, I'd expect this kind of proxying to be used mainly on services, whereas readonly classes would be mostly value objects. So the problem space may be smaller than it initially appears. --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: Issues with readonly classes
On Sat, Sep 10, 2022, at 4:34 PM, Nicolas Grekas wrote:
> Hello Larry,
>
>
>> 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?
>>
>> I don't have a real-world example, but a general pattern. The advantage
>> of a readonly object (either conventionally like PSR-7 or enforced by the
>> `readonly` keyword) is that you can store a reference to it without having
>> to worry about it changing out from under you. That means you can code on
>> the assumption that any data derived from that object is also not subject
>> to change.
>>
>> If you accept a readonly object of type Foo, you would write code around
>> it on that assumption.
>>
>> If you're then passed an object of type Bar extends Foo, which has an
>> extra non-readonly property, that assumption would now be broken. Whether
>> that counts as a Liskov violation is... a bit squishy, I think.
>>
>> Where that might be a problem is a case like this:
>>
>> readonly class Person {
>> public function __construct(public string $first, public string $last) {}
>>
>> public function fullName() {
>> return "$this->first $this->last";
>> }
>> }
>>
>> class FancyPerson extends Person {
>> public string $middle = '';
>>
>> public function setMiddle(string $mid) {
>> $this->middle = $mid;
>> }
>>
>> public function fullName() {
>> return "$this->first $this-middle $this->last";
>> }
>> }
>>
>> class Display {
>> public function __construct(protected Person $person) {}
>>
>> public function hello() {
>> return "Hello " . $this->person->fullName();
>> }
>> }
>>
>> Now, Display assumes Person is readonly, and thus fullName() is
>> idempotent, and thus Display::hello() is idempotent. But if you pass a
>> FancyPerson to Display, then fullName() is not safely idempotent (it may
>> change out from under you) and thus hello() is no longer idempotent.
>>
>> Whether or not that problem actually exists in practice is another
>> question. And to be fair, the same risk exists for conventionally-readonly
>> classes today (PSR-7, PSR-13, etc.), unless they're made final. Arguably
>> the safety signaling of a lexically readonly class is stronger than for a
>> conventionally readonly class, so one would expect that to not be broken.
>> But that's the case where a non-readonly child of a readonly parent can
>> cause problems.
>>
>
> Thanks for constructing this example, that's good food for thoughts.
> Unfortunately, The code following readonly child class shows that this
> safety you describe doesn't exist:
>
> readonly class FancyPerson extends Person {
> private readonly stdClass $middle;
>
> public function setMiddle(string $mid) {
> $this->middle ??= new stdClass;
> $this->middle->name = $mid;
> }
>
> public function fullName() {
> return "$this->first $this-middle $this->last";
> }
> }
Hm. I seem to recall during the discussion of readonly classes someone saying
that object properties of a readonly class had to also be readonly classes,
which would render the above code a compile error. However, I just checked and
that is not in the RFC. Was it removed? Am I imagining things? Anyone else
know what I'm talking about? :-)
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: Issues with readonly classes
Hello Larry,
> 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?
>
> I don't have a real-world example, but a general pattern. The advantage
> of a readonly object (either conventionally like PSR-7 or enforced by the
> `readonly` keyword) is that you can store a reference to it without having
> to worry about it changing out from under you. That means you can code on
> the assumption that any data derived from that object is also not subject
> to change.
>
> If you accept a readonly object of type Foo, you would write code around
> it on that assumption.
>
> If you're then passed an object of type Bar extends Foo, which has an
> extra non-readonly property, that assumption would now be broken. Whether
> that counts as a Liskov violation is... a bit squishy, I think.
>
> Where that might be a problem is a case like this:
>
> readonly class Person {
> public function __construct(public string $first, public string $last) {}
>
> public function fullName() {
> return "$this->first $this->last";
> }
> }
>
> class FancyPerson extends Person {
> public string $middle = '';
>
> public function setMiddle(string $mid) {
> $this->middle = $mid;
> }
>
> public function fullName() {
> return "$this->first $this-middle $this->last";
> }
> }
>
> class Display {
> public function __construct(protected Person $person) {}
>
> public function hello() {
> return "Hello " . $this->person->fullName();
> }
> }
>
> Now, Display assumes Person is readonly, and thus fullName() is
> idempotent, and thus Display::hello() is idempotent. But if you pass a
> FancyPerson to Display, then fullName() is not safely idempotent (it may
> change out from under you) and thus hello() is no longer idempotent.
>
> Whether or not that problem actually exists in practice is another
> question. And to be fair, the same risk exists for conventionally-readonly
> classes today (PSR-7, PSR-13, etc.), unless they're made final. Arguably
> the safety signaling of a lexically readonly class is stronger than for a
> conventionally readonly class, so one would expect that to not be broken.
> But that's the case where a non-readonly child of a readonly parent can
> cause problems.
>
Thanks for constructing this example, that's good food for thoughts.
Unfortunately, The code following readonly child class shows that this
safety you describe doesn't exist:
readonly class FancyPerson extends Person {
private readonly stdClass $middle;
public function setMiddle(string $mid) {
$this->middle ??= new stdClass;
$this->middle->name = $mid;
}
public function fullName() {
return "$this->first $this-middle $this->last";
}
}
> Personally I'm undecided at the moment whether or not it should be
> allowed. I'm sympathetic to the "it's easier to disallow and allow later
> than vice versa" argument, but still not sure where I stand. The above at
> least gives a concrete example of where the problem would be.
>
If we want the safety you describe, we might want a stronger version of it.
Let's name it immutable. An immutable property/class would be like a
readonly property with the additional restriction that only an immutable
value could be assigned to it (scalars + array + immutable classes.) But
that's another topic.
Your example made me doubt for a moment, but without any convincing
purpose, this should help decide that child classes shouldn't have to be
readonly.
Nicolas
Re: [PHP-DEV] Re: Issues with readonly classes
On Mon, Sep 5, 2022, at 4:01 AM, Máté Kocsis wrote:
> 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?
I don't have a real-world example, but a general pattern. The advantage of a
readonly object (either conventionally like PSR-7 or enforced by the `readonly`
keyword) is that you can store a reference to it without having to worry about
it changing out from under you. That means you can code on the assumption that
any data derived from that object is also not subject to change.
If you accept a readonly object of type Foo, you would write code around it on
that assumption.
If you're then passed an object of type Bar extends Foo, which has an extra
non-readonly property, that assumption would now be broken. Whether that
counts as a Liskov violation is... a bit squishy, I think.
Where that might be a problem is a case like this:
readonly class Person {
public function __construct(public string $first, public string $last) {}
public function fullName() {
return "$this->first $this->last";
}
}
class FancyPerson extends Person {
public string $middle = '';
public function setMiddle(string $mid) {
$this->middle = $mid;
}
public function fullName() {
return "$this->first $this-middle $this->last";
}
}
class Display {
public function __construct(protected Person $person) {}
public function hello() {
return "Hello " . $this->person->fullName();
}
}
Now, Display assumes Person is readonly, and thus fullName() is idempotent, and
thus Display::hello() is idempotent. But if you pass a FancyPerson to Display,
then fullName() is not safely idempotent (it may change out from under you) and
thus hello() is no longer idempotent.
Whether or not that problem actually exists in practice is another question.
And to be fair, the same risk exists for conventionally-readonly classes today
(PSR-7, PSR-13, etc.), unless they're made final. Arguably the safety
signaling of a lexically readonly class is stronger than for a conventionally
readonly class, so one would expect that to not be broken. But that's the case
where a non-readonly child of a readonly parent can cause problems.
Personally I'm undecided at the moment whether or not it should be allowed.
I'm sympathetic to the "it's easier to disallow and allow later than vice
versa" argument, but still not sure where I stand. The above at least gives a
concrete example of where the problem would be.
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] Re: Issues with readonly classes
Hi Mate 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. >> > > 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. > Thanks for the precision, works for me As I mentioned, there are ways around anyway (storing in another class or in a static variable.) 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.) > > > 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? > Not only can I not find where removing this invariant would be a problem, I also cannot think of where this invariant would be useful. I don't see what I can build on top of it... > 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. > During cloning only, yes, and we don't need any new syntax to allow this (we might need one to make it easier to write wither APIs, but that's another topic.) That's exactly what I'm trying to achieve in https://github.com/php/php-src/pull/9403. For the record, I tried 3 approaches to manage the transient state that is required to flag readonly properties as once-writeable during cloning. I tried using zval.u1 but I realized this is a read-only area right?, then tried using zval.u2.extra but this is not a bitfield, then zval.u2.property_guard but without luck. I didn't see any blocker either in the engine, so the only reason why I failed is because my C-related skills are too weak and I'd need help to figure out a proper patch. Nicolas >
[PHP-DEV] Re: Issues with readonly classes
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 point > as it's always possible to store shared global state in another class. But > 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 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.) 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áté
