Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hey Tim, Thank you. It would probably make sense to create a separate copy of the > current version of the branch, possibly creating separate PR and then to > drop the unrelated commits from #9497 - or alternatively picking the > first 4 commits and creating a new clean PR from that. The latter is > probably preferable. There's a large number of comments in the PR itself > that do not relate to the feature-as-proposed-in-the-RFC. > OK, I've just splitted the implementation of the two functionalities, and submitted https://github.com/php/php-src/pull/10389, containing only the related changes. I'm seeing that Nicolas already spelled out the unset() part in > __clone(), but I think it would also be useful to have that within the > example code. It's more easily missed in text. Examples are cheap :-) > I've also changed the example to include the unset case as well. :) Best regards Máté
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hi On 1/20/23 17:54, Máté Kocsis wrote: Sorry for the confusion! The PR contains an implementation for the "clone with" indeed just because it builds on top of some specifics of the 2nd proposal in the "Readonly amendments" RFC. However, the first few (4) commits are related to allowing modification of readonly properties in __clone(). Thank you. It would probably make sense to create a separate copy of the current version of the branch, possibly creating separate PR and then to drop the unrelated commits from #9497 - or alternatively picking the first 4 commits and creating a new clean PR from that. The latter is probably preferable. There's a large number of comments in the PR itself that do not relate to the feature-as-proposed-in-the-RFC. I'm seeing that Nicolas already spelled out the unset() part in __clone(), but I think it would also be useful to have that within the example code. It's more easily missed in text. Examples are cheap :-) Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hi Claude, Tim,
One shortcoming around readonly classes that I just figured out, is that it
> is not possible to use them as anonymous class:
>
Nice catch! As you wrote, it's indeed an oversight of the readonly class
implementation. Fortunately, we already have a PR with the fix. :)
I'm confused by the linked PRs for the implementation, because they do
> not appear to match what's proposed. Specifically the one for proposal
> #2 (allow modification in __clone) appears to include unrelated changes
> (a clone-with{} syntax).
>
Sorry for the confusion! The PR contains an implementation for the "clone
with" indeed
just because it builds on top of some specifics of the 2nd proposal in the
"Readonly amendments" RFC. However,
the first few (4) commits are related to allowing modification of readonly
properties in __clone().
Máté Kocsis
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hi
On 1/19/23 09:01, Máté Kocsis wrote:
As discussion apparently stalled, and since we managed to update the RFC
with the recently brought up arguments, we would like to start the vote
soon, possibly early next week, unless someone finds a new topic to discuss.
I'm confused by the linked PRs for the implementation, because they do
not appear to match what's proposed. Specifically the one for proposal
#2 (allow modification in __clone) appears to include unrelated changes
(a clone-with{} syntax).
Also proposal #2 should be explicit about the behavior of unset(). It
appears with the current implementation using unset() within __clone()
is allowed and does what you expect it to do.
Best regards
Tim Düsterhus
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
> Le 19 janv. 2023 à 10:12, Claude Pache a écrit :
>
>
>
>> Le 19 janv. 2023 à 09:01, Máté Kocsis a écrit :
>>
>> Hi Everyone,
>>
>> As discussion apparently stalled, and since we managed to update the RFC
>> with the recently brought up arguments, we would like to start the vote
>> soon, possibly early next week, unless someone finds a new topic to discuss.
>>
>> Máté
>>
>
> Hi,
>
> One shortcoming around readonly classes that I just figured out, is that it
> is not possible to use them as anonymous class:
>
> ```php
> $c = new readonly class { }; // parse error
> ```
>
> While it does not makes sense to have `abstract` and it is not useful to have
> `final` in this position, on the other hand, it is perfectly reasonable to
> have readonly here. (This is especially problematic in case we keep the
> limitation that only readonly classes may extend readonly classes, because in
> that case, an anonymous class could not extend a readonly class. However,
> this is an orthogonal concern.)
>
> —Claude
As I think that this limitation is most probably a bug and not a deliberate
decision (the RFC introducing readonly classes did not mention restrictions
around anonymous classes), I have open a bug report:
https://github.com/php/php-src/issues/10377
—Claude
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
> Le 19 janv. 2023 à 09:01, Máté Kocsis a écrit :
>
> Hi Everyone,
>
> As discussion apparently stalled, and since we managed to update the RFC
> with the recently brought up arguments, we would like to start the vote
> soon, possibly early next week, unless someone finds a new topic to discuss.
>
> Máté
>
Hi,
One shortcoming around readonly classes that I just figured out, is that it is
not possible to use them as anonymous class:
```php
$c = new readonly class { }; // parse error
```
While it does not makes sense to have `abstract` and it is not useful to have
`final` in this position, on the other hand, it is perfectly reasonable to have
readonly here. (This is especially problematic in case we keep the limitation
that only readonly classes may extend readonly classes, because in that case,
an anonymous class could not extend a readonly class. However, this is an
orthogonal concern.)
—Claude
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hi Everyone, As discussion apparently stalled, and since we managed to update the RFC with the recently brought up arguments, we would like to start the vote soon, possibly early next week, unless someone finds a new topic to discuss. Máté Larry Garfield ezt írta (időpont: 2022. nov. 30., Sze, 20:35): > On Wed, Nov 30, 2022, at 9:46 AM, Deleu wrote: > > After reading GPB, Nicolas, Jordan and Larry's considerations, I no > longer > > have any objections to this RFC. Here is my summary of it all: > > > > - It's very easy for everyone to wrongly interpret readonly as somewhat > > immutable, but it isn't (docs/education issue) > > - LSP is about the writer of the child class, not about PHP > > - If you don't want child classes to violate LSP, make your class `final > > readonly` > > - readonly as "constructor-init" properties mindset make it even stronger > > the argument that child classes should be free to choose their definition > > because constructors are special methods not bound by inheritance. > > Just for the record, my whole point is that "readonly as constructor-init" > is wrong, and I am angry at the SA tools that have invented that out of > whole cloth because it just breaks workflows that I am using very > effectively and safely. I always turn off that check in those because they > are wrong. That is *not* how the language feature is implemented, so > making other language feature decisions based on that incorrect, artificial > "rule" is highly dangerous. > > --Larry Garfield > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On Wed, Nov 30, 2022, at 9:46 AM, Deleu wrote: > After reading GPB, Nicolas, Jordan and Larry's considerations, I no longer > have any objections to this RFC. Here is my summary of it all: > > - It's very easy for everyone to wrongly interpret readonly as somewhat > immutable, but it isn't (docs/education issue) > - LSP is about the writer of the child class, not about PHP > - If you don't want child classes to violate LSP, make your class `final > readonly` > - readonly as "constructor-init" properties mindset make it even stronger > the argument that child classes should be free to choose their definition > because constructors are special methods not bound by inheritance. Just for the record, my whole point is that "readonly as constructor-init" is wrong, and I am angry at the SA tools that have invented that out of whole cloth because it just breaks workflows that I am using very effectively and safely. I always turn off that check in those because they are wrong. That is *not* how the language feature is implemented, so making other language feature decisions based on that incorrect, artificial "rule" is highly dangerous. --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
After reading GPB, Nicolas, Jordan and Larry's considerations, I no longer have any objections to this RFC. Here is my summary of it all: - It's very easy for everyone to wrongly interpret readonly as somewhat immutable, but it isn't (docs/education issue) - LSP is about the writer of the child class, not about PHP - If you don't want child classes to violate LSP, make your class `final readonly` - readonly as "constructor-init" properties mindset make it even stronger the argument that child classes should be free to choose their definition because constructors are special methods not bound by inheritance. Overall the most important aspect for me was that there was a pragmatic decision of supporting readonly properties as it covers most of the need without the whole hustle/complexity of asymmetric visibility and it was very good for Nicolas to bring up again that pragmatic notion. This doesn't have to be complex and limiting child classes makes it more complex. In the future, the education around readonly keyword will be that it has no bearings anywhere except the class that makes use of the keyword. -- Marco Deleu
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On Sat, 26 Nov 2022 at 23:06, Marco Pivetta wrote:
> On Sat, 26 Nov 2022, 23:56 G. P. B., wrote:
>
>> As all the invariants will be maintained in the child classes
>>
> The invariant here is the immutability from a behavioural PoV.
> LSP has 100% been broken in that example.
>
I've gone back to the example, and if we are going to use bad code to
"argue" against certain features, then let's use this argument to its
natural conclusion.
But before that, we really need to get everyone clear on what LSP is.
It is a subtyping relationship that states:
if a property P on an object X of type T is provable to be true, then for
an object Y of type S to be considered a proper subtype of T, this property
P must also be true for all object Y of type S.
Let's rephrase this slightly in terms more appropriate to PHP:
if a characteristic P on an instance X of class T is provable to be true,
then for an instance Y of class S to be considered a proper subclass of T,
this characteristic P must also be true for all instances Y of class S.
Having this in mind, let me show how easy it is to break LSP:
class T {
public function __construct(public readonly int $i) {}
public function add(self $t): self { return new self($this->i + $t->i);
}
}
$op1 = new T(5);
$op2 = new T(14);
$r = $op1->add($op2);
var_dump($r);
class S extends T {
public function add(parent $t): self { return new self($this->i -
$t->i); }
}
$op1 = new S(5);
$op2 = new T(14);
$r = $op1->add($op2);
var_dump($r);
Clearly, the characteristic that we can "add two T together" has been
broken in class S.
This is all to say, the type system can only provide a level of code
*correctness*, and that remains even in highly sophisticated type systems.
If you bear with me and the introduction of the following imaginary type
dependent syntax: Matrix which declares a Matrix type which is an n by
m matrix.
Thus, we can write the signature of a function that does define matrix
multiplication in the following way:
function multiplication(Matrix $op1, Matrix $op2): Matrix
Which would guarantee us that the 2 matrices provided can indeed be
multiplied together as they have compatible dimensions, and it even gives
us the dimensions of the output matrix.
However, the one thing it cannot guarantee is that the operation we are
actually doing is a matrix multiplication, I could just as well write a
function that returns a new n by k matrix filled with zeroes.
How can one be sure that a subclass will not alter the behaviour? By making
the method final.
That's mainly why personally I think final by default, and using
composition over inheritance, is "better".
And I think from what we've seen in language design over the last couple of
decades is the OO inheritance is just a bad way to do subtyping and
something like Haskell type-classes are way better. [1] But that ship has
somewhat sailed for PHP.
The LSP checks PHP (and for the matter any type system) can do and does, is
checking the logical correctness of the implementation, not behavioural
correctness that onus still remains on the developer.
Now, let's have another look at your example:
/* readonly */ class ImmutableCounter {
public function __construct(private readonly int $count) {}
public function add1(): self { return new self($this->count + 1); }
public function value(): int { return $this->count; }
}
IMHO the fact that the count property is private is the biggest issue, just
changing it to protected would solve most of the issues, as now you've
given the tools to PHP to verify and ensure this constraint holds for the
whole class hierarchy.
But more importantly, it means I don't need to reimplement the *whole*
parent class and (risk me messing up)/(keeping in sync) the behaviour of
the parent class if I want to do something like:
class SubstractableImmutableCounter extends ImmutableCounter {
public function sub1(): self { return new self($this->count - 1); }
}
As it's easy to mess up even just writing a function properly that does the
expected behaviour, I suppose we should remove the ability for users to
write them, as they cannot be trusted with.
So please, can we stop using the argument that this proposal breaks LSP if
you write dumb code, as that's already the case, and it doesn't if you
provide PHP with the actual information it needs to verify in child classes.
Moreover, if one expects a readonly class to be immutable, then that is
already not the case if one of the properties is a non-readonly object.
It is also not necessarily stateless, as can be seen in the following
example: https://3v4l.org/A2Lbs
readonly class ImmutableBadNumberGenerator {
public function __construct(private readonly int $i) {}
public function generate(): int {
static $j = 1;
++$j;
return $this->i + $j;
}
}
$o = new ImmutableBadNumberGenerator(10);
var_dump($o->generate());
var_dump($o->generate());
var_dump($o->generate());
Now what *is* up to debate is what
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
The code that Marco (Pivetta) shared was supposed to illustrate how
readonly classes can be useful to enforce some invariants on child classes.
Yet, here is another implementation that does use a readonly child class,
but still provides the behavior that was supposedly prevented by the
keyword:
readonly class MutableCounter extends ImmutableCounter {
private stdClass $count;
public function __construct(int $count) {
$this->count = (object) ['count' => $count];
}
public function add1(): self { return new self(++$this->count->count); }
public function value(): int { return $this->count->count; }
}
This counterexample shows that readonly classes do not provide any extra
safeguards.
As it stands, the readonly keyword on classes has two consequences:
1. it forces writing dummy boilerplate to work around the current
limitation while failing to provide any real guarantees
2. it gives false expectations - the exact one that we're discussions about
here (no, readonly classes don't help enforce any aspects of LSP)
1. is why I call the current restriction arbitrary, and 2. might be
dangerous since it would make people build on non-existing grounds.
If we stay with the current way, we'll just have another WTF in the
language IMHO. One that will make it harder to master PHP.
Le dim. 27 nov. 2022 à 17:51, Larry Garfield a
écrit :
> On Sat, Nov 26, 2022, at 6:35 PM, Jordan LeDoux wrote:
> > On Sat, Nov 26, 2022 at 3:40 PM Deleu wrote:
> >
> >>
> >> As I think more about this, there's nothing about the current RFC in
> this
> >> code sample. What's breaking LSP here is the child class doing state
> >> modification, not PHP. To further expand that rationale, PHP allows us
> to
> >> create child classes. Whether that class will be LSP-safe or not is up
> to
> >> us, not up to PHP.
> >>
> >> However, the point still stands. Allowing child classes to break
> readonly
> >> will make it easier to build code that breaks LSP. The question then
> >> becomes: why is this being proposed and is it worth it?
> >>
> >
> > I cannot help but feel that the way `readonly` is being treated is going
> to
> > end up one of those things that is regretted. "Readonly does not imply
> > immutability". The fact that very nearly *every* single person who has
> not
> > worked on the RFCs has at some point been confused by this however should
> > be very telling.
> >
> > This comes from two *different* avenues that compound with each other to
> > *both* make this design head-scratching to me.
> >
> > First, in virtually all other technical contexts where the term
> "readonly"
> > is used, it means that the information/data cannot be altered. That is
> not
> > the case with readonly. In PHP, in this implementation, it is not
> > "readonly" in the sense that it is used everywhere else for computing, it
> > is "assign once".
> >
> > Second, the English words "read only", particularly to native speakers,
> > make this behavior very counterintuitive and confusing. I won't belabor
> > that point further.
> >
> > What "read only" really is, is "constructor initialize only". It honestly
> > has nothing to do with "read" as it's implemented.
>
> Not quite. It really is just write-once. The idea that you can only do
> that in the constructor is not in the language; that's been invented by
> over-eager static analysis tools. (Everyone should disable that check.)
>
> > I guess I worry that this RFC makes `readonly` even more of a minefield
> for
> > PHP developers, increasing the mental load of using it in code while
> *even
> > further* watering down the benefits it may provide. It's already designed
> > in a somewhat counterintuitive way that I feel will be almost completely
> > replaced in actual code in the wild by "immutable" if PHP ever gets that.
>
> Working on asymmetric visibility, I have come to agree. Nikita proposed
> readonly as a "junior version" of asymmetric visibility, to cover the most
> common use case without introducing more complexity. At the time, he was
> confident that it wouldn't preclude expanding to asymmetric visibility in
> the future. Well... I can say with confidence at this point that is not
> correct, and the design of readonly is causing issues for asymmetric
> visibility, and for cloning, to the point that (based on feedback in the
> other thread) we're likely going to for now forbid readonly and a-viz on
> the same property.
>
> At this point, I think I view readonly as a cautionary tale about the
> costs of doing "quick and easy" design over something more robust, because
> the quick-and-easy creates problems down the line that a more thoughtful,
> holistic view would have avoided.
>
To me, the best outcome of this discussion should be to retire readonly
*classes*. They were thought as a quick way to not repeat the readonly
keyword on every property, but they in fact introduce this behavior +
expectations related to child classes and these collide. From a conceptual
ground, I
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On Sat, Nov 26, 2022, at 6:35 PM, Jordan LeDoux wrote: > On Sat, Nov 26, 2022 at 3:40 PM Deleu wrote: > >> >> As I think more about this, there's nothing about the current RFC in this >> code sample. What's breaking LSP here is the child class doing state >> modification, not PHP. To further expand that rationale, PHP allows us to >> create child classes. Whether that class will be LSP-safe or not is up to >> us, not up to PHP. >> >> However, the point still stands. Allowing child classes to break readonly >> will make it easier to build code that breaks LSP. The question then >> becomes: why is this being proposed and is it worth it? >> > > I cannot help but feel that the way `readonly` is being treated is going to > end up one of those things that is regretted. "Readonly does not imply > immutability". The fact that very nearly *every* single person who has not > worked on the RFCs has at some point been confused by this however should > be very telling. > > This comes from two *different* avenues that compound with each other to > *both* make this design head-scratching to me. > > First, in virtually all other technical contexts where the term "readonly" > is used, it means that the information/data cannot be altered. That is not > the case with readonly. In PHP, in this implementation, it is not > "readonly" in the sense that it is used everywhere else for computing, it > is "assign once". > > Second, the English words "read only", particularly to native speakers, > make this behavior very counterintuitive and confusing. I won't belabor > that point further. > > What "read only" really is, is "constructor initialize only". It honestly > has nothing to do with "read" as it's implemented. Not quite. It really is just write-once. The idea that you can only do that in the constructor is not in the language; that's been invented by over-eager static analysis tools. (Everyone should disable that check.) > I guess I worry that this RFC makes `readonly` even more of a minefield for > PHP developers, increasing the mental load of using it in code while *even > further* watering down the benefits it may provide. It's already designed > in a somewhat counterintuitive way that I feel will be almost completely > replaced in actual code in the wild by "immutable" if PHP ever gets that. Working on asymmetric visibility, I have come to agree. Nikita proposed readonly as a "junior version" of asymmetric visibility, to cover the most common use case without introducing more complexity. At the time, he was confident that it wouldn't preclude expanding to asymmetric visibility in the future. Well... I can say with confidence at this point that is not correct, and the design of readonly is causing issues for asymmetric visibility, and for cloning, to the point that (based on feedback in the other thread) we're likely going to for now forbid readonly and a-viz on the same property. At this point, I think I view readonly as a cautionary tale about the costs of doing "quick and easy" design over something more robust, because the quick-and-easy creates problems down the line that a more thoughtful, holistic view would have avoided. > LSP doesn't exist because it is some objectively better way of programming > according to universal laws of entropy or something. It is instead > important because LSP helps programmers be able to predict the behavior of > the program they are writing and reduces the short-term memory load > involved in programming and architecture. > > Something that *technically* complies with LSP but makes the program harder > to predict and increases the mental load of programming violates the > *purpose* of LSP. We can argue about whether it is technically correct, but > I feel like that somewhat misses the point: making the language more > capable, more stable, and more predictable. > > In other words, I do not believe it is that important or care to argue > about whether this RFC violates LSP. It violates the *purpose* of LSP, and > that's a bigger problem to me personally. > > Jordan At this point, I am inclined to agree. readonly is wonky enough as is. Making it semi-LSP (in spirit) just makes it even more wonky. To flip the earlier sentiment, "readonly is broken enough as is, let's not break it even further." --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On Sat, Nov 26, 2022 at 3:40 PM Deleu wrote: > > As I think more about this, there's nothing about the current RFC in this > code sample. What's breaking LSP here is the child class doing state > modification, not PHP. To further expand that rationale, PHP allows us to > create child classes. Whether that class will be LSP-safe or not is up to > us, not up to PHP. > > However, the point still stands. Allowing child classes to break readonly > will make it easier to build code that breaks LSP. The question then > becomes: why is this being proposed and is it worth it? > I cannot help but feel that the way `readonly` is being treated is going to end up one of those things that is regretted. "Readonly does not imply immutability". The fact that very nearly *every* single person who has not worked on the RFCs has at some point been confused by this however should be very telling. This comes from two *different* avenues that compound with each other to *both* make this design head-scratching to me. First, in virtually all other technical contexts where the term "readonly" is used, it means that the information/data cannot be altered. That is not the case with readonly. In PHP, in this implementation, it is not "readonly" in the sense that it is used everywhere else for computing, it is "assign once". Second, the English words "read only", particularly to native speakers, make this behavior very counterintuitive and confusing. I won't belabor that point further. What "read only" really is, is "constructor initialize only". It honestly has nothing to do with "read" as it's implemented. I guess I worry that this RFC makes `readonly` even more of a minefield for PHP developers, increasing the mental load of using it in code while *even further* watering down the benefits it may provide. It's already designed in a somewhat counterintuitive way that I feel will be almost completely replaced in actual code in the wild by "immutable" if PHP ever gets that. LSP doesn't exist because it is some objectively better way of programming according to universal laws of entropy or something. It is instead important because LSP helps programmers be able to predict the behavior of the program they are writing and reduces the short-term memory load involved in programming and architecture. Something that *technically* complies with LSP but makes the program harder to predict and increases the mental load of programming violates the *purpose* of LSP. We can argue about whether it is technically correct, but I feel like that somewhat misses the point: making the language more capable, more stable, and more predictable. In other words, I do not believe it is that important or care to argue about whether this RFC violates LSP. It violates the *purpose* of LSP, and that's a bigger problem to me personally. Jordan
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On Thu, Nov 17, 2022 at 11:47 AM Marco Pivetta wrote:
> ```php
>
> /* readonly */ class ImmutableCounter {
> public function __construct(private readonly int $count) {}
> public function add1(): self { return new self($this->count + 1); }
> public function value(): int { return $this->count; }
> }
>
> // assuming the proposed RFC is in place, this is now possible:
> class MutableCounter extends ImmutableCounter {
> public function __construct(private int $count) {}
> public function add1(): self { return new self(++$this->count); }
> public function value(): int { return $this->count; }
> }
>
> $counter1 = new ImmutableCounter(0);
> $counter2 = $counter1->add1();
> $counter3 = $counter2->add1();
>
> var_dump([$counter1->value(), $counter2->value(), $counter3->value()]);
>
> $mutableCounter1 = new MutableCounter(0);
> $mutableCounter2 = $mutableCounter1->add1();
> $mutableCounter3 = $mutableCounter2->add1();
>
> var_dump([$mutableCounter1->value(), $mutableCounter2->value(),
> $mutableCounter3->value()]);
> ```
>
> ( https://3v4l.org/IDhRY )
>
> That's really really confusing, buggy, and, from a consumer PoV, broken
> (LSP violation too, transitively).
>
As I think more about this, there's nothing about the current RFC in this
code sample. What's breaking LSP here is the child class doing state
modification, not PHP. To further expand that rationale, PHP allows us to
create child classes. Whether that class will be LSP-safe or not is up to
us, not up to PHP.
However, the point still stands. Allowing child classes to break readonly
will make it easier to build code that breaks LSP. The question then
becomes: why is this being proposed and is it worth it?
As I read through the RFC, the only reason this is being proposed is
because "it breaks decoration via inheritance". Doesn't `final` cause the
same issue? How is this problem being handled if the class is marked as
final? If you can't extend a final class to build your "decoration via
inheritance", why can't the same argument be applied to readonly classes?
--
Marco Deleu
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On Sat, 26 Nov 2022, 23:56 G. P. B., wrote: > > As all the invariants will be maintained in the child classes > > The invariant here is the immutability from a behavioural PoV. LSP has 100% been broken in that example. > >
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On Thu, 17 Nov 2022 at 14:47, Marco Pivetta wrote:
> Hey Máté,
>
> On Tue, 15 Nov 2022 at 07:30, Máté Kocsis wrote:
>
> > Hi Everyone,
> >
> > Following Nicolas' thread about "Issues with readonly classes" (
> > https://externals.io/message/118554), we created an RFC to fix two
> issues
> > with the readonly behavior: https://wiki.php.net/rfc/readonly_amendments
> >
> >
> Since I was reviewing https://github.com/lcobucci/jwt/pull/979 today, I
> had
> to put the "why immutability is an LSP requirement" in examples.
>
> As explained in
> https://github.com/lcobucci/jwt/pull/979#discussion_r1025280053, a
> consumer
> relying on a `readonly` implementation of a class probably also expects
> replacement implementations as read-only.
> I am aware that we lack the `readonly` marker at interface level (would be
> really neat), but the practical example is as follows:
>
>
> ```php
>
> /* readonly */ class ImmutableCounter {
> public function __construct(private readonly int $count) {}
> public function add1(): self { return new self($this->count + 1); }
> public function value(): int { return $this->count; }
> }
>
> // assuming the proposed RFC is in place, this is now possible:
> class MutableCounter extends ImmutableCounter {
> public function __construct(private int $count) {}
> public function add1(): self { return new self(++$this->count); }
> public function value(): int { return $this->count; }
> }
>
> $counter1 = new ImmutableCounter(0);
> $counter2 = $counter1->add1();
> $counter3 = $counter2->add1();
>
> var_dump([$counter1->value(), $counter2->value(), $counter3->value()]);
>
> $mutableCounter1 = new MutableCounter(0);
> $mutableCounter2 = $mutableCounter1->add1();
> $mutableCounter3 = $mutableCounter2->add1();
>
> var_dump([$mutableCounter1->value(), $mutableCounter2->value(),
> $mutableCounter3->value()]);
> ```
>
> This prints ( https://3v4l.org/IDhRY )
>
> ```
> array(4) {
> [0]=>
> int(0)
> [1]=>
> int(1)
> [2]=>
> int(2)
> }
> array(4) {
> [0]=>
> int(1)
> [1]=>
> int(2)
> [2]=>
> int(2)
> }
> ```
>
> I took a simplified example on purpose, just to demonstrate the problem
> with `readonly` disappearing in child classes.
>
> That's really really confusing, buggy, and, from a consumer PoV, broken
> (LSP violation too, transitively).
>
Removing the readonly restriction for a child class is *not* an LSP
violation.
As all the invariants will be maintained in the child classes (i.e. all the
properties of the parent class remain readonly).
Moreover, a consumer of the base class can only rely on the API defined by
said class, as such it will not be aware of any mutable properties defined
by a child class.
LSP is really not about the class hierarchy/subtypes itself, but the
relation of input data and output data of the operations allowed on a type
(i.e. method calls or property access), and because readonly is not the
same as immutability there is no theoretical issue.
Now, I can agree that it can be slightly surprising that a class which has
a writeable property extends from one which only has readonly ones, but one
can already immitate this by declaring every property readonly but not the
class.
Best regards,
George P. Banyard
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On Sat, 26 Nov 2022, 20:45 Máté Kocsis, wrote: > We proposed this change because it wouldn't break anything that's already > not "broken". > That's quite a terrible rationale, TBH >
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On Sat, Nov 26, 2022, 4:45 PM Máté Kocsis wrote: > We proposed this change because it wouldn't break anything that's already > not "broken". > > Regards: > Máté > The example provided already raised some eyebrows from people. I think the argument of "let's furthet enhance this 'broken' behavior" isn't that great. I find the ability of child class to ignore the parent readonly definition awkward at best. >
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hi Marco,
Sorry for the very late reply!
As explained in
> https://github.com/lcobucci/jwt/pull/979#discussion_r1025280053, a
> consumer relying on a `readonly`
>
implementation of a class probably also expects replacement implementations
> as read-only.
> I am aware that we lack the `readonly` marker at interface level (would be
> really neat), but the practical example is as follows:
>
Regarding readonly interfaces: it would be a good idea given properties
were be able to be declared in interfaces. Then readonly
interfaces could adopt the same semantics as readonly classes have, so that
all *declared* properties become readonly.
Regarding the LSP violation: your example is about immutability (as the
name of the classes also suggest), but the readonly
keyword doesn't guarantee immutability. This is basically what Nicolas
answered not long ago to Larry:
https://externals.io/message/118554#118604
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";
> }
> }
We proposed this change because it wouldn't break anything that's already
not "broken".
Regards:
Máté
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hey Máté,
On Tue, 15 Nov 2022 at 07:30, Máté Kocsis wrote:
> Hi Everyone,
>
> Following Nicolas' thread about "Issues with readonly classes" (
> https://externals.io/message/118554), we created an RFC to fix two issues
> with the readonly behavior: https://wiki.php.net/rfc/readonly_amendments
>
>
Since I was reviewing https://github.com/lcobucci/jwt/pull/979 today, I had
to put the "why immutability is an LSP requirement" in examples.
As explained in
https://github.com/lcobucci/jwt/pull/979#discussion_r1025280053, a consumer
relying on a `readonly` implementation of a class probably also expects
replacement implementations as read-only.
I am aware that we lack the `readonly` marker at interface level (would be
really neat), but the practical example is as follows:
```php
count + 1); }
public function value(): int { return $this->count; }
}
// assuming the proposed RFC is in place, this is now possible:
class MutableCounter extends ImmutableCounter {
public function __construct(private int $count) {}
public function add1(): self { return new self(++$this->count); }
public function value(): int { return $this->count; }
}
$counter1 = new ImmutableCounter(0);
$counter2 = $counter1->add1();
$counter3 = $counter2->add1();
var_dump([$counter1->value(), $counter2->value(), $counter3->value()]);
$mutableCounter1 = new MutableCounter(0);
$mutableCounter2 = $mutableCounter1->add1();
$mutableCounter3 = $mutableCounter2->add1();
var_dump([$mutableCounter1->value(), $mutableCounter2->value(),
$mutableCounter3->value()]);
```
This prints ( https://3v4l.org/IDhRY )
```
array(4) {
[0]=>
int(0)
[1]=>
int(1)
[2]=>
int(2)
}
array(4) {
[0]=>
int(1)
[1]=>
int(2)
[2]=>
int(2)
}
```
I took a simplified example on purpose, just to demonstrate the problem
with `readonly` disappearing in child classes.
That's really really confusing, buggy, and, from a consumer PoV, broken
(LSP violation too, transitively).
That said, allowing mutation in `__clone()` is fine.
Marco Pivetta
https://twitter.com/Ocramius
https://ocramius.github.io/
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On 15 November 2022 19:23:44 GMT, "Máté Kocsis" wrote: >I didn't bother to add a separate example for explicit readonly properties, >because I mainly regard >readonly classes as a shorthand for adding the readonly modifier for all >properties My two cents: examples are cheap, misunderstandings are expensive. What you have now is like a test suite that only tests the happy path - the example doesn't demonstrate any of the details of the RFC. It would be really helpful to have an example, or a commented line of code in a bigger example, for each of the things this RFC will let you do, and some that it *won't* let you do, even for things that seem obvious to you, because they might not be obvious to everyone. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hi Rowan and Alexandru, I'm not totally clear what this sentence is saying: > > > Reinitialization can only take place /during/ the |__clone()| magic > method call, no matter if the actual assignment happens in a different > method or function invoked inside |__clone()|. > > Are you saying that invoking a separate method like > "setSomeProperty($blah)" from __clone() is allowed, or that it isn't? > Perhaps a couple of extra examples could be added to this section > demonstrating what would and wouldn't be allowed? > Sorry, I tried hard to articulate the behavior, but then the wording still needs some clarification. So this sentence was intended to convey that properties can be reinitialized during the execution of the __clone() magic method either if the assignment happens directly in this method, or in any method invoked by __clone(). Is it clearer now? Thanks for pointing this problem out, and any help with the wording is appreciated! For proposal 2 part: "Readonly properties can be reinitialized during > cloning" > From the details there I understand it would affect properties in a > readonly class as well as explicit readonly properties in non-readonly class. Is that the intent? If yes, > maybe we can share an example without a readonly class as well. I didn't bother to add a separate example for explicit readonly properties, because I mainly regard readonly classes as a shorthand for adding the readonly modifier for all properties (even though I know there are some other differences between readonly and non-readonly classes). That's why I don't think it's worth adding two different examples, but I'm ok with having one example with simply readonly properties, without the readonly class modifier. I think the example will be clear enough this way. Regards: Máté
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hi Mate, On Tue, Nov 15, 2022 at 8:30 AM Máté Kocsis wrote: > Hi Everyone, > > Following Nicolas' thread about "Issues with readonly classes" ( > https://externals.io/message/118554), we created an RFC to fix two issues > with the readonly behavior: https://wiki.php.net/rfc/readonly_amendments > > Please let us know your thoughts! > For proposal 2 part: "Readonly properties can be reinitialized during cloning" >From the details there I understand it would affect properties in a readonly class as well as explicit readonly properties in non-readonly class. Is that the intent? If yes, maybe we can share an example without a readonly class as well. Regards, Alex
Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments
On 15/11/2022 06:30, Máté Kocsis wrote: Hi Everyone, Following Nicolas' thread about "Issues with readonly classes" ( https://externals.io/message/118554), we created an RFC to fix two issues with the readonly behavior:https://wiki.php.net/rfc/readonly_amendments Please let us know your thoughts! Máté Hi Máté, I'm not totally clear what this sentence is saying: > Reinitialization can only take place /during/ the |__clone()| magic method call, no matter if the actual assignment happens in a different method or function invoked inside |__clone()|. Are you saying that invoking a separate method like "setSomeProperty($blah)" from __clone() is allowed, or that it isn't? Perhaps a couple of extra examples could be added to this section demonstrating what would and wouldn't be allowed? Regards, -- Rowan Tommins [IMSoP]
[PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hi Everyone, Following Nicolas' thread about "Issues with readonly classes" ( https://externals.io/message/118554), we created an RFC to fix two issues with the readonly behavior: https://wiki.php.net/rfc/readonly_amendments Please let us know your thoughts! Máté
