On Wed, Aug 25, 2021, at 10:28 AM, Sara Golemon wrote: > On Wed, Aug 25, 2021 at 5:03 AM Nikita Popov <nikita....@gmail.com> wrote: > > I'd like to propose the deprecation of "dynamic properties", that is > > properties that have not been declared in the class (stdClass and > > __get/__set excluded, of course): > > > > https://wiki.php.net/rfc/deprecate_dynamic_properties > > > > > This RFC offers `extends stdClass` as a way to opt-in to the use of > dynamic properties. > > > This makes the assumption that existing uses of dynamic properties are all > root classes. I don't think that assumption can be made. > > > Some people have suggested that we could use a magic marker > > interface (implements SupportsDynamicProperties), > > an attribute (#[SupportsDynamicProperties]) > > or a trait (use DynamicProperties;) instead. > > > My gut-check says an Attribute works well enough. This will unlock the > class (disable deprecation warning) in 8.2+ and be ignored in earlier > releases (8.0 and 8.1 would need an auto-loaded polyfill userspace > attribute obviously, but that's a tractable problem). > > #[SupportsDynamicProperties] > class Foo { ... } > > > The Locked classes RFC took an alternative approach to this problem space: > > Rather than deprecating/removing dynamic properties and providing an > opt-in > > for specific classes, it instead allowed marking specific classes as > locked in > > order to forbid creation of dynamic properties on them. > > > > I don't believe that this is the right strategy, because in contemporary > code, > > classes being “locked” is the default state, while classes that require > dynamic > > properties are a rare exception. Additionally, this requires that class > owners > > (which may be 3rd party packages) consistently add the “locked” keyword > > to be effective. > > > I struggle here, because the opt-in approach is the most BC, but I actually > think you're right. I think[citation needed] that dynamic props are > probably rare enough that as long as the escape hatch is clear, we can be a > little more bold about nudging the ecosystem forward. I will counter > however, that the same issue with 3rd party libraries applies to opt-out as > opt-in. A third party library that's undermaintained (and uses dynamic > props) won't be able to be used out of the box once we apply this. I don't > think that's enough to scare us off, it just means that the opt-in side of > that argument cancels out. > > -Sara
I am overall in favor of this move and making declared-only the default. What I'm still undecided on is what escape hatch should be allowed. I agree that "you can extend stdClass already" isn't a good one, because that isn't viable in many cases. The most viable options in my mind are "use __get/__set yourself, you heathen" and an attribute. The benefit of an attribute over an interface or new parent class is that it's silently backward compatible, especially since attributes are comments in <8.0 versions, so legacy code can add that marker and still be compatible with anything from 5.0 on. The "internal impact" section makes me lean toward the "__get/__set" option, but I don't fully understand the benefits of the impact. It seems it would make the engine a little tidier and more efficient, but I don't have a good sense for how much more efficient, which would need to be balanced against how much more INefficient user-space code with dynamic properties would become. (Either using __get/__set itself, or inheriting from a future stdClass that implements them internally.) IMO we should prioritize engine efficiency over edge-case efficiency (which dynamic properties are these days), but the amount still matters. I'm currently working on some improvements to the FIG to allow it to release small, incomplete code libraries or shared type libraries. Perhaps a fully user-space BC trait with the basic behavior (basically what's shown in the RFC) could live there? (Not speaking on FIG's behalf, here, just musing aloud.) --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
