RFC: Locked Classes

  104620
March 10, 2019 18:35 rowan.collins@gmail.com (Rowan Collins)
Hi all,

I'd like to present a new RFC for "locked classes": classes which 
restrict dynamically adding or removing properties from their instances.

While it can be useful, the ability to set an object property which is 
not part of the class definition can also lead to subtle bugs. Banning 
this for all objects would be a significant and painful breaking change, 
so I propose instead the option to mark a particular class with a new 
keyword, "locked".

An instance of a locked class behaves like any other object, except that:

- Attempting to set a property on the instance which was not declared in 
the class (or inherited from one of its parent classes) will throw an 
error, and the instance will not be modified.
- Attempting to read a property on the instance which was not declared 
(or inherited) will throw an error, rather than raising a Notice and 
evaluating to null.
- Attempting to call unset() on any property of the instance will throw 
an error, and the instance will not be modified.

Note that ECMAScript  / JavaScript includes a similar feature, called 
"sealed objects". However, the proposed modifier applies to classes, and 
"sealed class" has a different meaning elsewhere (e.g. C#, Kotlin), so 
I've chosen "locked class" to avoid confusion.

For further details and examples, please check the RFC at 
https://wiki.php.net/rfc/locked-classes and the tests in the draft 
implementation at https://github.com/php/php-src/pull/3931

Regards,

-- 
Rowan Collins
[IMSoP]
  104622
March 10, 2019 18:51 ocramius@gmail.com (Marco Pivetta)
Hi Rowan,

Overall good idea, except that I disagree with the `unset()` being
disabled: that bit is strictly required for lazy-loading purposes, and
mostly harmless for userland ("normal" people don't do it, libraries do it)..

Besides that (blocker for me), if this RFC would be enforced in my coding
standards of choice 👍

Does anybody know if this has potential for engine optimizations?


On Sun, 10 Mar 2019, 19:35 Rowan Collins, collins@gmail.com> wrote:

> Hi all, > > I'd like to present a new RFC for "locked classes": classes which > restrict dynamically adding or removing properties from their instances. > > While it can be useful, the ability to set an object property which is > not part of the class definition can also lead to subtle bugs. Banning > this for all objects would be a significant and painful breaking change, > so I propose instead the option to mark a particular class with a new > keyword, "locked". > > An instance of a locked class behaves like any other object, except that: > > - Attempting to set a property on the instance which was not declared in > the class (or inherited from one of its parent classes) will throw an > error, and the instance will not be modified. > - Attempting to read a property on the instance which was not declared > (or inherited) will throw an error, rather than raising a Notice and > evaluating to null. > - Attempting to call unset() on any property of the instance will throw > an error, and the instance will not be modified. > > Note that ECMAScript / JavaScript includes a similar feature, called > "sealed objects". However, the proposed modifier applies to classes, and > "sealed class" has a different meaning elsewhere (e.g. C#, Kotlin), so > I've chosen "locked class" to avoid confusion. > > For further details and examples, please check the RFC at > https://wiki.php.net/rfc/locked-classes and the tests in the draft > implementation at https://github.com/php/php-src/pull/3931 > > Regards, > > -- > Rowan Collins > [IMSoP] > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
  104624
March 10, 2019 19:23 rowan.collins@gmail.com (Rowan Collins)
Hi Marco,

On 10/03/2019 18:51, Marco Pivetta wrote:
> Overall good idea, except that I disagree with the `unset()` being > disabled: that bit is strictly required for lazy-loading purposes, and > mostly harmless for userland ("normal" people don't do it, libraries > do it).
To me, the unset() restriction fits naturally into the definition; I think most people would consider "removing a property" in the same category as "adding a property". Notably, it's included in ECMAScript's definition of sealed objects [1], which is essentially the same feature. I understand that unset() currently enables a neat trick for lazy-loading, but it feels like just that to me: a trick, not a core part of the language. Similarly, there may be tricks that rely on adding dynamic properties to objects, but that's fine, just don't mark those classes as "locked". [1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/seal Regards, -- Rowan Collins [IMSoP]
  104626
March 10, 2019 19:32 larry@garfieldtech.com ("Larry Garfield")
On Sun, Mar 10, 2019, at 1:52 PM, Marco Pivetta wrote:
> Hi Rowan, > > Overall good idea, except that I disagree with the `unset()` being > disabled: that bit is strictly required for lazy-loading purposes, and > mostly harmless for userland ("normal" people don't do it, libraries do it). > > Besides that (blocker for me), if this RFC would be enforced in my coding > standards of choice 👍 > > Does anybody know if this has potential for engine optimizations?
Marco, have you an example of such lazy-loading tricks? I'm not familiar with them but would like to be... I'm overall +1 here as well, as I am for most things that allow me to make my code stricter and pickier. (I don't have a strong feeling on unset() either way.) --Larry Garfield
  104628
March 10, 2019 19:46 ocramius@gmail.com (Marco Pivetta)
Typical scenario (works for either of the properties - note that typed
properties that aren't nullable and have no default value also behave the
same way):

class Foo {
    private int $a;
    private $b;
    public function __construct() {
        unset($this->b);
    }
    public function __get(string $name) {
        $this->a = 123
        $this->b = 'abc';
        return $this->$name;
    }
    public function saySomething() : string {
        return $this->a . $this->b;
    }
}

echo (new Foo())->saySomething();

https://3v4l.org/Rn39Q (note: 3v4l doesn't have 7.4, so I simplified it a
bit)

@rowan: I've stated it multiple times in the past, but if the mechanism is
to be removed, then it should be replaced with a language-level concept of
laziness then.

On Sun, 10 Mar 2019, 20:32 Larry Garfield, <larry@garfieldtech.com> wrote:

> On Sun, Mar 10, 2019, at 1:52 PM, Marco Pivetta wrote: > > Hi Rowan, > > > > Overall good idea, except that I disagree with the `unset()` being > > disabled: that bit is strictly required for lazy-loading purposes, and > > mostly harmless for userland ("normal" people don't do it, libraries do > it). > > > > Besides that (blocker for me), if this RFC would be enforced in my coding > > standards of choice 👍 > > > > Does anybody know if this has potential for engine optimizations? > > Marco, have you an example of such lazy-loading tricks? I'm not familiar > with them but would like to be... > > I'm overall +1 here as well, as I am for most things that allow me to make > my code stricter and pickier. (I don't have a strong feeling on unset() > either way.) > > --Larry Garfield > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
  104633
March 10, 2019 20:19 rowan.collins@gmail.com (Rowan Collins)
On 10/03/2019 19:46, Marco Pivetta wrote:
> @rowan: I've stated it multiple times in the past, but if the mechanism is > to be removed, then it should be replaced with a language-level concept of > laziness then.
1) I'm all for adding a language-level concept of laziness. It feels like it could fit with a syntax for getters and setters, which has been discussed a few times, but never quite made it. 2) I'm not saying that the current behaviour should be removed, I'm saying that dynamically unsetting properties is inherently incompatible with declaring a class "locked". I've actually had more than one suggestion that __get(), __set(), and __unset() should not be allowed at all on locked classes; in which case, it wouldn't matter how unset() behaved, because the lazy-loading trick relies on __get() as well. Regards, -- Rowan Collins [IMSoP]
  104623
March 10, 2019 18:56 gadelat@gmail.com (Gabriel O)
Isn’t unset() currently only way to remove reference? This part is hence very problematic.

> On 10. Mar 2019, at 19:35, Rowan Collins collins@gmail.com> wrote: > > Hi all, > > I'd like to present a new RFC for "locked classes": classes which restrict dynamically adding or removing properties from their instances. > > While it can be useful, the ability to set an object property which is not part of the class definition can also lead to subtle bugs. Banning this for all objects would be a significant and painful breaking change, so I propose instead the option to mark a particular class with a new keyword, "locked". > > An instance of a locked class behaves like any other object, except that: > > - Attempting to set a property on the instance which was not declared in the class (or inherited from one of its parent classes) will throw an error, and the instance will not be modified. > - Attempting to read a property on the instance which was not declared (or inherited) will throw an error, rather than raising a Notice and evaluating to null. > - Attempting to call unset() on any property of the instance will throw an error, and the instance will not be modified. > > Note that ECMAScript / JavaScript includes a similar feature, called "sealed objects". However, the proposed modifier applies to classes, and "sealed class" has a different meaning elsewhere (e.g. C#, Kotlin), so I've chosen "locked class" to avoid confusion. > > For further details and examples, please check the RFC at https://wiki.php.net/rfc/locked-classes and the tests in the draft implementation at https://github.com/php/php-src/pull/3931 > > Regards, > > -- > Rowan Collins > [IMSoP] > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
  104627
March 10, 2019 19:36 rowan.collins@gmail.com (Rowan Collins)
On 10/03/2019 18:56, Gabriel O wrote:
> Isn’t unset() currently only way to remove reference? This part is hence very problematic.
Hm, that's an interesting case to consider, thanks! At the moment, the following code breaks the reference between $a->foo and $bar, but also deletes the entire property from the instance: class A { public $foo; } $a = new A; $a->foo =& $bar; unset($a->foo); var_dump($a->foo); # Notice: Undefined property: A::$foo This feels very peculiar to me: $foo clearly *is* a defined property, and I didn't want to over-ride the class definition, I just wanted to "reset" that property. We can chalk it up to "references are bad", but it is indeed problematic that there isn't an alternative way to say "breakReference($a->foo);" that doesn't have this side effect. Regards, -- Rowan Collins [IMSoP]
  104631
March 10, 2019 20:16 pmjones88@gmail.com (Paul Jones)
> On Mar 10, 2019, at 13:35, Rowan Collins collins@gmail.com> wrote: > > Hi all, > > I'd like to present a new RFC for "locked classes": classes which restrict dynamically adding or removing properties from their instances.
Nice. This would help enforce at least one element of immutability in userland; namely, not having to define `final public function __set()` and `final public function __unset()` to prevent adding and mutating undefined properties. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  104632
March 10, 2019 20:19 Danack@basereality.com (Dan Ackroyd)
On Sun, 10 Mar 2019 at 18:35, Rowan Collins collins@gmail.com> wrote:
> > Hi all, > > - Attempting to set a property on the instance which was not declared in > the class (or inherited from one of its parent classes) will throw an > error, and the instance will not be modified. > - Attempting to read a property on the instance which was not declared > (or inherited) will throw an error, rather than raising a Notice and > evaluating to null.
I believe those two parts of the RFC are completely possible already in user-land with a trait similar to the one below. What would be the compelling case for making it be a keyword, rather than the user-land implementation that is already achievable? Also.....this seems to be a special case of an idea that has come up repeatedly, of allowing the concept of 'packages' with rules applied to all of the classes in a package e.g. * class Friendship which only allows access to certain methods to other classes in the same package. * strict enabled by default for a classes in a package. and now this RFC (or at least 2/3 of it) could be implemented by adding some traits by default to all classes in a package. Rather than adding a special case, I'd much rather we looked at providing the general case. cheers Dan Ack trait LockedClass { public function __set($name, $value) { throw new \Exception("Property [$name] doesn't exist for class [" . get_class($this) . "] so can't set it"); } public function __get($name) { throw new \Exception("Property [$name] doesn't exist for class [" . get_class($this) . "] so can't get it"); } }
  104634
March 10, 2019 20:48 rowan.collins@gmail.com (Rowan Collins)
On 10/03/2019 20:19, Dan Ackroyd wrote:
> I believe those two parts of the RFC are completely possible already > in user-land with a trait similar to the one below. What would be the > compelling case for making it be a keyword, rather than the user-land > implementation that is already achievable?
Hi Dan, Yes, such a trait had occurred to me before. My main motivation for proposing it as part of the language is to make it as easy as possible to opt into, and standardising the mechanics - I can see "all classes must be strict" making it into far more coding standards than "all projects must include this Trait and add it to all classes". It occurred to me that language support might also allow optimisation around it, whereas implementing __get()/__set() would likely do the opposite. Apart from that, I was partly just experimenting with how hard it would be to add; the answer, so far at least, is surprisingly easy. The alternative would be to simply deprecate dynamic property getting and setting altogether. I suspect that would affect a lot of code, though, so would be wary of actually removing it as soon as PHP 8, given that the set part doesn't even raise a notice right now.
> and now this RFC (or at least 2/3 of it) could be implemented by > adding some traits by default to all classes in a package.
This wasn't really relevant to my motivation - you'd still have to add it to each class under the proposed RFC - but it is an interesting idea. In general, I think such packages would be a great addition to the language, but they'd also be a big change in how things work, and take a lot of working out, so I'm not going to hold my breath. Regards, -- Rowan Collins [IMSoP]
  104635
March 10, 2019 20:56 rowan.collins@gmail.com (Rowan Collins)
On 10/03/2019 20:19, Dan Ackroyd wrote:
> trait LockedClass > { > public function __set($name, $value) > { > throw new \Exception("Property [$name] doesn't exist for class [" . > get_class($this) . "] so can't set it"); > } > public function __get($name) > { > throw new \Exception("Property [$name] doesn't exist for class [" . > get_class($this) . "] so can't get it"); > } > }
Incidentally, this isn't quite equivalent to the current RFC, because it will also intercept access to private or protected properties from out of scope. It doesn't make much difference other than making the error messages less helpful, but it's an example of how a nice userland implementation isn't quite as trivial as it first looks. Regards, -- Rowan Collins [IMSoP]
  104636
March 10, 2019 21:09 ocramius@gmail.com (Marco Pivetta)
Still feasible though: PRs to
https://github.com/Roave/Dont/tree/master/src/Dont welcome

On Sun, 10 Mar 2019, 21:56 Rowan Collins, collins@gmail.com> wrote:

> On 10/03/2019 20:19, Dan Ackroyd wrote: > > trait LockedClass > > { > > public function __set($name, $value) > > { > > throw new \Exception("Property [$name] doesn't exist for class [" . > > get_class($this) . "] so can't set it"); > > } > > public function __get($name) > > { > > throw new \Exception("Property [$name] doesn't exist for class [" . > > get_class($this) . "] so can't get it"); > > } > > } > > > Incidentally, this isn't quite equivalent to the current RFC, because it > will also intercept access to private or protected properties from out > of scope. It doesn't make much difference other than making the error > messages less helpful, but it's an example of how a nice userland > implementation isn't quite as trivial as it first looks. > > Regards, > > -- > Rowan Collins > [IMSoP] > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
  104639
March 10, 2019 22:37 rowan.collins@gmail.com (Rowan Collins)
On 10/03/2019 21:09, Marco Pivetta wrote:
> Still feasible though: PRs to > https://github.com/Roave/Dont/tree/master/src/Dont welcome
Indeed. Many things in a language are sugar for something that could be done in a more complex way; it's a judgement call which ones are worth including, and maybe this one is, maybe it's not. Note that that trait would definitely break lazy-loading, though, because it declares a final __get() Regards, -- Rowan Collins [IMSoP]
  104648
March 11, 2019 11:27 nikita.ppv@gmail.com (Nikita Popov)
On Sun, Mar 10, 2019 at 7:35 PM Rowan Collins collins@gmail.com>
wrote:

> Hi all, > > I'd like to present a new RFC for "locked classes": classes which > restrict dynamically adding or removing properties from their instances. > > While it can be useful, the ability to set an object property which is > not part of the class definition can also lead to subtle bugs. Banning > this for all objects would be a significant and painful breaking change, > so I propose instead the option to mark a particular class with a new > keyword, "locked". > > An instance of a locked class behaves like any other object, except that: > > - Attempting to set a property on the instance which was not declared in > the class (or inherited from one of its parent classes) will throw an > error, and the instance will not be modified. > - Attempting to read a property on the instance which was not declared > (or inherited) will throw an error, rather than raising a Notice and > evaluating to null. > - Attempting to call unset() on any property of the instance will throw > an error, and the instance will not be modified. > > Note that ECMAScript / JavaScript includes a similar feature, called > "sealed objects". However, the proposed modifier applies to classes, and > "sealed class" has a different meaning elsewhere (e.g. C#, Kotlin), so > I've chosen "locked class" to avoid confusion. > > For further details and examples, please check the RFC at > https://wiki.php.net/rfc/locked-classes and the tests in the draft > implementation at https://github.com/php/php-src/pull/3931
I like the general idea of this, though I think it's not quite the right way to go about solving the problem. The issue I see with this proposal is that it requires annotating each class with an additional keyword -- however, chances are good that people will want all *classes* to be locked, including those they do not have direct control over (coming from libraries). The solution I would prefer is the ability to declare that within a project, all interactions with objects (whether they are my own or come from 3rd parties) should disallow the creation of dynamic properties. This differs from your proposal in two important points: 1. You cannot create dynamic properties on objects even if they come from 3rd-party code you do not control. 2. 3rd-party code may interact with your objects (and it's own) however it likes. It is not affected by the disabling of dynamic properties in your project code. If this sounds familiar, this is basically strict_types, but for dynamic properties. Of course, doing this with declares as they are is rather cumbersome, because you'd have to specify them in every single file. Things would be different if you could specify a declare for a whole project... This is what https://wiki.php.net/rfc/namespace_scoped_declares was about, and it actually happens to use exactly this example as one of the things the mechanism could be used for. I still think that this would be the more principled approach to this issue, especially as it easily extends to other problem areas, without requiring a new keyword for each. (Based on previous feedback, it should probably be directory-scope declares rather than namespace-scope.) Now, to get back to your RFC, two notes: 1. The BC break section is not terribly clear due to the use of "semi-reserved". I initially assumed that this was a contextual keyword (i.e. only has special meaning immediately before "class") but apparently this is a normal reserved keyword in the implementation (i.e. no classes, functions, namespaces etc named "locked"). 2. It is possible to break a reference by reassigning it to another reference, like so: $null = null; $this->prop =& $null; unset($null); This is not quite as good as unsetting because it leaves behind an rc=1 reference, but that would be the way to remove a reference without unset(). Regards, Nikita
  104649
March 11, 2019 12:14 rowan.collins@gmail.com (Rowan Collins)
Hi Nikita,

On Mon, 11 Mar 2019 at 11:27, Nikita Popov ppv@gmail.com> wrote:

> The solution I would prefer is the ability to declare that within a > project, all interactions with objects (whether they are my own or come > from 3rd parties) should disallow the creation of dynamic properties. This > differs from your proposal in two important points: > 1. You cannot create dynamic properties on objects even if they come from > 3rd-party code you do not control. > 2. 3rd-party code may interact with your objects (and it's own) however > it likes. It is not affected by the disabling of dynamic properties in your > project code. >
That's an interesting approach. I think it's conceptually a lot more complicated (and probably more complicated to implement, too) - the same object would behave in different ways in different contexts. There are also edge cases to define which the current proposal doesn't need to worry about, like stdClass, and objects which were created in one scope and manipulated in another: declare(strict_properties=0) { class Foo {} $foo = new Foo; $foo->bar = 42; } declare(strict_properties=1) { echo $foo; # Is $bar now 'defined', or do we only count the properties in the original class definition? } More importantly, you would have to check that third-party code was declaring the properties you needed before enabling it. For instance, a library might document that you should set `$handler->debug = true`, read that property internally, but never declare it; you'd then have to patch the library before you could run in strict_properties mode. It feels like at that point, we might as well just deprecate dynamic properties altogether - which I'm beginning to think is the best approach. To put it explicitly, what are the use cases where you'd turn such a flag off?
> 1. The BC break section is not terribly clear due to the use of > "semi-reserved". I initially assumed that this was a contextual keyword > (i.e. only has special meaning immediately before "class") but apparently > this is a normal reserved keyword in the implementation (i.e. no classes, > functions, namespaces etc named "locked"). >
Oops, I'll hold my hands up to not actually testing that. I think I took that wording from a comment in the parser, but probably didn't read it properly. 2. It is possible to break a reference by reassigning it to another
> reference, like so: > > $null = null; > $this->prop =& $null; > unset($null); > > This is not quite as good as unsetting because it leaves behind an rc=1 > reference, but that would be the way to remove a reference without unset(). >
Huh, cunning. :) Regards, -- Rowan Collins [IMSoP]
  104651
March 11, 2019 12:32 rowan.collins@gmail.com (Rowan Collins)
On Mon, 11 Mar 2019 at 12:14, Rowan Collins collins@gmail.com> wrote:

> > declare(strict_properties=0) { > class Foo {} > $foo = new Foo; > $foo->bar = 42; > } > declare(strict_properties=1) { > echo $foo; # Is $bar now 'defined', or do we only count the properties > in the original class definition? > } >
Oops, that should read echo $foo->bar; -- Rowan Collins [IMSoP]
  104652
March 11, 2019 12:35 nikita.ppv@gmail.com (Nikita Popov)
On Mon, Mar 11, 2019 at 1:14 PM Rowan Collins collins@gmail.com>
wrote:

> Hi Nikita, > > On Mon, 11 Mar 2019 at 11:27, Nikita Popov ppv@gmail.com> wrote: > > > The solution I would prefer is the ability to declare that within a > > project, all interactions with objects (whether they are my own or come > > from 3rd parties) should disallow the creation of dynamic properties. > This > > differs from your proposal in two important points: > > 1. You cannot create dynamic properties on objects even if they come > from > > 3rd-party code you do not control. > > 2. 3rd-party code may interact with your objects (and it's own) however > > it likes. It is not affected by the disabling of dynamic properties in > your > > project code. > > > > > That's an interesting approach. I think it's conceptually a lot more > complicated (and probably more complicated to implement, too) - the same > object would behave in different ways in different contexts. There are also > edge cases to define which the current proposal doesn't need to worry > about, like stdClass, and objects which were created in one scope and > manipulated in another: > > declare(strict_properties=0) { > class Foo {} > $foo = new Foo; > $foo->bar = 42; > } > declare(strict_properties=1) { > echo $foo; # Is $bar now 'defined', or do we only count the properties > in the original class definition? > } >
I'd expect it to be defined, especially as it allows interoperating with libraries that are missing property declarations and set dynamic properties (intentionally). More importantly, you would have to check that third-party code was
> declaring the properties you needed before enabling it. For instance, a > library might document that you should set `$handler->debug = true`, read > that property internally, but never declare it; you'd then have to patch > the library before you could run in strict_properties mode. > > It feels like at that point, we might as well just deprecate dynamic > properties altogether - which I'm beginning to think is the best approach. > > To put it explicitly, what are the use cases where you'd turn such a flag > off? >
You'd turn it off in the case you mentioned: A 3rd-party library that relies on dynamic properties. The idea is that you disable dynamic properties globally for your own code, but can opt back in for cases where they are necessary for some reason (most likely interoperability, but possibly also code doing weird Marco things). This allows you to single out some piece of code as "here be dragons", rather than keeping dragons as the default state :) Removing the ability to use dynamic properties (even excluding stdClass) is probably not feasible on a short timetime. However, I think that having a directory-scoped declare would also be useful for that, because it allows a more gradual migration. You can start with having strict_properties default to false and have interested parties opt-in. Then you can flip the default to true, while still giving people who rely heavily on it the ability to opt-out. Removal would be the final stage where the ability to opt-out goes away. Regarding optimizations that your RFC allows: In theory we could drop the properties_table member (8 bytes) for objects of locked classes. In practice this is probably not beneficial as access to the embedded property table would no longer be at a fixed offset. Nikita
> 1. The BC break section is not terribly clear due to the use of > > "semi-reserved". I initially assumed that this was a contextual keyword > > (i.e. only has special meaning immediately before "class") but apparently > > this is a normal reserved keyword in the implementation (i.e. no classes, > > functions, namespaces etc named "locked"). > > > > > Oops, I'll hold my hands up to not actually testing that. I think I took > that wording from a comment in the parser, but probably didn't read it > properly. > > > > 2. It is possible to break a reference by reassigning it to another > > reference, like so: > > > > $null = null; > > $this->prop =& $null; > > unset($null); > > > > This is not quite as good as unsetting because it leaves behind an rc=1 > > reference, but that would be the way to remove a reference without > unset(). > > > > > Huh, cunning. :) > > Regards, > -- > Rowan Collins > [IMSoP] >
  104655
March 11, 2019 14:18 rowan.collins@gmail.com (Rowan Collins)
On Mon, 11 Mar 2019 at 12:35, Nikita Popov ppv@gmail.com> wrote:

> > Removing the ability to use dynamic properties (even excluding stdClass) > is probably not feasible on a short timetime. However, I think that having > a directory-scoped declare would also be useful for that, because it allows > a more gradual migration. You can start with having strict_properties > default to false and have interested parties opt-in. Then you can flip the > default to true, while still giving people who rely heavily on it the > ability to opt-out. Removal would be the final stage where the ability to > opt-out goes away. > >
I think that's actually the best argument I've seen so far. The current proposal is basically very conservative: a simple, self-contained change, that even a noob like me was able to implement. The downside of that is that it doesn't have a long-term vision: there's no clear path to making all classes "locked" by default. I agree that the combination of package- or directory-scoped declares, and a switch that can be both opt-in and opt-out, gives a better future direction. I just hope we get those things, rather than just having neither - as happened with "static class", which was rejected partly because "we'll have function autoloading real soon". Regards, -- Rowan Collins [IMSoP]
  104684
March 12, 2019 21:40 smalyshev@gmail.com (Stanislav Malyshev)
Hi!

> While it can be useful, the ability to set an object property which is > not part of the class definition can also lead to subtle bugs. Banning > this for all objects would be a significant and painful breaking change, > so I propose instead the option to mark a particular class with a new > keyword, "locked".
Isn't it just: trait Locked { public function __set($name, $value) { throw new RuntimeException("Property $name not declared!"); } public function __get($name) { throw new RuntimeException("Property $name not declared!"); } public function __unset($name) { throw new RuntimeException("Property $name not declared!"); } } The RFC says: While this can be achieved through strategic use of the __set, __get, and __unset magic methods, this is long-winded, hard to optimise, and interferes with other uses of those methods. I don't see how it's "long-winded" - it's just three one-liners, I don't see how "interferes with other uses" of these methods (if you want other uses, just add code to the method), and as for optimization, I think we already have optimization for pre-declared properties, and if you don't use any others, it would work just as well. -- Stas Malyshev smalyshev@gmail.com
  104685
March 12, 2019 21:45 gadelat@gmail.com (Gabriel O)
It was already addressed how using such trivial solution is insufficient, please read whole thread

> On 12. Mar 2019, at 22:40, Stanislav Malyshev <smalyshev@gmail.com> wrote: > > Hi! > >> While it can be useful, the ability to set an object property which is >> not part of the class definition can also lead to subtle bugs. Banning >> this for all objects would be a significant and painful breaking change, >> so I propose instead the option to mark a particular class with a new >> keyword, "locked". > > Isn't it just: > > trait Locked { > public function __set($name, $value) { > throw new RuntimeException("Property $name not declared!"); > } > public function __get($name) { > throw new RuntimeException("Property $name not declared!"); > } > public function __unset($name) { > throw new RuntimeException("Property $name not declared!"); > } > } > > The RFC says: > > While this can be achieved through strategic use of the __set, __get, > and __unset magic methods, this is long-winded, hard to optimise, and > interferes with other uses of those methods. > > I don't see how it's "long-winded" - it's just three one-liners, I don't > see how "interferes with other uses" of these methods (if you want other > uses, just add code to the method), and as for optimization, I think we > already have optimization for pre-declared properties, and if you don't > use any others, it would work just as well. > > > -- > Stas Malyshev > smalyshev@gmail.com > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
  104688
March 12, 2019 23:33 rowan.collins@gmail.com (Rowan Collins)
On 12/03/2019 21:40, Stanislav Malyshev wrote:
> Isn't it just: > trait Locked { > public function __set($name, $value) { > throw new RuntimeException("Property $name not declared!"); > } > public function __get($name) { > throw new RuntimeException("Property $name not declared!"); > } > public function __unset($name) { > throw new RuntimeException("Property $name not declared!"); > } > }
Almost... Magic methods are run both for undefined properties and out-of-scope ones, so at the very least this leads to unhelpful or vague error messages. It also doesn't handle unset in the same way as the current RFC, because it won't affect unsetting a property that was previously defined (although that part of the RFC is slightly controversial, it seems). There may be other edge cases it doesn't handle the same way an internal implementation would, as well, I'm not sure. It's certainly not a major saving over hacking it together in userland, but nor is it a major weight to add it to the language. I think having a well-thought-out implementation built in (either the one in the current RFC, or a declare directive as has been suggested elsewhere) would encourage people to use it, and generally be more user-friendly. Regards, -- Rowan Collins [IMSoP]