Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Hi Juliette, On Mon, 15 Nov 2021 at 23:36, wrote: > > I've been asked to post the link to the Twitter discussion in this > thread for visibility. > > The Twitter thread generated, and is still generating, quite a lot of > discussion, I'm not going to quote from the Twitter thread partly as lot of that discussion isn't that pleasant. To be clear, this change isn't being proposed to annoy open source maintainers, it's proposed because people think it will make the language better, to a great enough value to be worth the BC break. But also, a lot of that thread is about the experience of being an open source developer is absolutely terrible due to many factors including: * an almost complete refusal of companies to sponsor work. * people who work for companies that use open source having an attitude of entitlement to 'gold level support', and will very quickly start using emotionally manipulative language e.g. "if you cared about this project you'd work harder on it". * a lack of new contributors to open source. I completely understand how all of things are pretty aggravating. It's particularly galling when a maintainer tries to get some funding, e.g. by holding back a release targetting a new version of PHP, other people will undermine that effort by forking the library and doing just enough to get it working, but not committing to do any future work. I'm in the lucky position that because PECL is so hard to use (and gate-keepered as to who can use it), that I was able to hold off doing the release of Imagick with PHP 8.0 support until a couple of companies stepped up with a (greatly appreciated) non-trivial amount of sponsorship. What the future holds for open source is unclear to me. And it's not at all obvious that open source isn't a morally wrong thing to do, as to a large extent it seems to rely on individuals subsidising for profit companies. However I don't think that slowing down the improvements in PHP core itself is an appropriate response to companies refusing to pay to support the projects they rely as a business on. I do recommend anyone who has an open source project to: * make sure you have github sponsors or other sponsoring services setup. * ask for payment for work done previously. If you've created a library that companies are dependent on, it's okay to refuse to work on it until there is a decent amount of support for that work done. You don't have to commit to doing new work until the project has a sustainable rate of sponsorship. * compare the amount of money you're getting for a project to the commercial rates for developers, and not to how much cup of coffee is. * Feel free to forward any cash you don't feel justified keeping to other opensource projects particularly upstream dependencies or tools. Or if you don't have enough time to work on that project yourself, feel free to ask for enough money to pay someone else to work on it. Once those done, it's perfectly acceptable to put a project on 'strike' until it's funded adequately and refuse to release new versions to accomodate changes in PHP. I know that saying 'no' to users is draining as they so often try to guilt maintainers into doing work for free. If anyone would like me to help explain to users "your company needs to start sponsoring this project before this project will acknowledge this issue", in any of their projects repos, please ping me on twitter https://twitter.com/MrDanack cheers Dan Ack -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Tue, Nov 16, 2021 at 5:55 PM Larry Garfield wrote: > 1. If we adopt the RFC right now as-is, the market has ~12 months to add > the attribute. If we instead have a default-true flag that changes to > default false in the future, it means at minimum 24 months in which to add > the attribute to opt-in to dynamic properties. > > Okay sure, but that's an argument about lead time, not about implementation. If we agree on targeting 9.0 for this becoming an error (and I think we do), then the argument that's left is whether users get notified as early as 8.2, or if they only get notified as of 8.3. Personally, I want to give users MORE lead time, but having the deprecation warnings come up sooner, rather than later. Assuming 9.0 comes out in late 2025 (guesstimate based on the cadence of 7.x), then including the deprecation with 8.2 (released in late 2022) will give users three years to attach an attribute where needed. If we delay the deprecation notice until 8.3 (released in late 2023), then they only have two years. I know this is the opposite end of lead time than you're talking about (you want max lead time before a deprecation notice even shows up), and here's why you're wrong: Most users don't pay attention to internals. That extra year you give them until notices show up will be wasted in ignorance as they don't know they have any work to do at all. All you will have done is robbed them of a year to implement the escape hatch (though let's be honest, they don't even need ONE year to do it). The only developers paying attention to internals@ traffic are the ones who have literally already added this attribute to their code bases in anticipation of this change. > 2. The RFC as-is has no way for developers to opt-in to actively rejecting > dynamic properties. They'll get a deprecation, but... we're shouting from > the rooftops that deprecations shouldn't be a big red warning, so if you > want a big red warning you can't get that until PHP 9. With the flag > attribute, developers could opt into "please slap me across the face if I > use dynamic properties by accident" in ~12 months when 8.2 ships, rather > than waiting 36-48 months until the likely PHP 9 release. > > Your premise is that, since deprecation warnings will be ignored, we should increase the verbosity level of the warning, but only for people who clearly know that a problem exists and can opt in to it? I'm sorry, but I don't track the logic of that. -Sara
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
> Le 17 nov. 2021 à 00:56, Larry Garfield a écrit : > > They'll get a deprecation, but... we're shouting from the rooftops that > deprecations shouldn't be a big red warning, so if you want a big red warning > you can't get that until PHP 9. Deprecation notices *are* big red warnings. They mean: the feature *will* be removed or altered in the next major version. The beauty of deprecation notices is that they warn you about future breakage without actually breaking anything. There is no need to invent some new sophisticated way, but only to learn to use correctly the existing one. —Claude -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Tue, Nov 16, 2021, at 4:10 PM, Sara Golemon wrote:
> On Mon, Nov 15, 2021 at 11:21 AM Larry Garfield
> wrote:
>
>> A possible idea to help make this transition (which I do support) more
>> gradual:
>>
>> Instead of an "allow" attribute, introduce a boolean flag attribute.
>>
>> #[DynamicProperties(true)]
>> class Beep {}
>>
>> The attribute marks whether dynamic properties are enabled or disabled via
>> boolean. If disabled, then they're an error if used.
>>
>> 8.2: Introduce the attribute, with a default of TRUE. Exactly zero code
>> breaks, but people can start adding the attribute now. That means people
>> who want to lock-out dynamic properties can do so starting now, and those
>> cases that do need them (or can't easily get away from them) can be flagged
>> far in advance.
>> 8.something: Change the default to FALSE. Using dynamic properties when
>> false throws a deprecation, not an error. People have had some number of
>> years to add the attribute either direction if desired.
>>
>
> This is exactly what Nikita is proposing, except that instead of the
> attribute being introduced in PHP 8.2, he's proposing to introduce it
> EARLIER. Roughly PHP 2 sort of timeframe.
>
> This is because attributes are forward compatible by design and developers
> can already start adding #[AllowDynamicProperties] to their code NOW. Even
> their code that was written to run cleanly on PHP 5.6 because users are
> terrible at upgrading their servers despite a 2x performance increase.
>
>
> The point is, we don't need 8.2 to be a soft-launch before deprecation
> because precisely nobody is prevented from adding this attribute
> preemptively.
>
> -Sara
It's not *quite* the same, although your point about preemptive attributes is
valid.
1. If we adopt the RFC right now as-is, the market has ~12 months to add the
attribute. If we instead have a default-true flag that changes to default
false in the future, it means at minimum 24 months in which to add the
attribute to opt-in to dynamic properties.
2. The RFC as-is has no way for developers to opt-in to actively rejecting
dynamic properties. They'll get a deprecation, but... we're shouting from the
rooftops that deprecations shouldn't be a big red warning, so if you want a big
red warning you can't get that until PHP 9. With the flag attribute,
developers could opt into "please slap me across the face if I use dynamic
properties by accident" in ~12 months when 8.2 ships, rather than waiting 36-48
months until the likely PHP 9 release.
So it gives the nitpickers what they want sooner, and gives the old-codies more
time to get their ducks in a row.
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Mon, Nov 15, 2021 at 11:21 AM Larry Garfield
wrote:
> A possible idea to help make this transition (which I do support) more
> gradual:
>
> Instead of an "allow" attribute, introduce a boolean flag attribute.
>
> #[DynamicProperties(true)]
> class Beep {}
>
> The attribute marks whether dynamic properties are enabled or disabled via
> boolean. If disabled, then they're an error if used.
>
> 8.2: Introduce the attribute, with a default of TRUE. Exactly zero code
> breaks, but people can start adding the attribute now. That means people
> who want to lock-out dynamic properties can do so starting now, and those
> cases that do need them (or can't easily get away from them) can be flagged
> far in advance.
> 8.something: Change the default to FALSE. Using dynamic properties when
> false throws a deprecation, not an error. People have had some number of
> years to add the attribute either direction if desired.
>
This is exactly what Nikita is proposing, except that instead of the
attribute being introduced in PHP 8.2, he's proposing to introduce it
EARLIER. Roughly PHP 2 sort of timeframe.
This is because attributes are forward compatible by design and developers
can already start adding #[AllowDynamicProperties] to their code NOW. Even
their code that was written to run cleanly on PHP 5.6 because users are
terrible at upgrading their servers despite a 2x performance increase.
The point is, we don't need 8.2 to be a soft-launch before deprecation
because precisely nobody is prevented from adding this attribute
preemptively.
-Sara
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Tue, Nov 16, 2021 at 6:00 PM Andreas Heigl wrote: > Hey list. > Which performance improvement of the "original state" of the RFC? As > that was one of the questions that were not 100% answered: What are the > benefits for the language. And while those 8 bit that Nikita mentioned > in the "Motivation" part of the RFC look nice, he also stated that "this > is a fairly long-term benefit that will require additional technical > work to realize". > I think these two messages might have some information about the performance improvements: https://externals.io/message/115800#115848 https://externals.io/message/115800#115872 Yes, maybe not everything was captured in the final RFC text. I also think all the evaluated options should have been present in the RFC as it brings more context to voters. Regards, Alex
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Hey list. On 16.11.21 16:26, Deleu wrote: On Tue, Nov 16, 2021 at 3:59 PM James Gilliland wrote: On Tue, Nov 16, 2021 at 4:23 AM Rowan Tommins wrote: On 16/11/2021 09:27, Andreas Heigl wrote: I see, yes, code that is 100% perfectly tested can get away without the language performing any error checking at all - the behaviour is all guaranteed by the tests. I would be very surprised if even 1% of PHP applications can claim such comprehensive tests. The topic here was that new code can verify the declaration of a property by using tests. That does not need to happen on the language level. I was never talking about adding tests for existing code. For most code bases, even new ones being written from scratch in PHP 8.0, that level of testing simply doesn't exist, and having the language tell you "hey, you wrote $this->loger instead of $this->logger" is a useful feature. And, in a lot of cases, more useful than having the language say "OK, I've created your dynamic $loger property for you", which is what currently happens. What you described there sounds like a warning and not a fatal error. Maybe that's where some of the trepidation is coming from. I know I'm less worried about the deprecation notice and more worried about what happens in PHP 9 when it's a fatal error. I can't say that this line of reasoning has its merits, but then there's no benefit to the engine itself. Issuing a warning and carry on materializing dynamic properties will never bring the original performance improvement that was part of the original state of the RFC. Which performance improvement of the "original state" of the RFC? As that was one of the questions that were not 100% answered: What are the benefits for the language. And while those 8 bit that Nikita mentioned in the "Motivation" part of the RFC look nice, he also stated that "this is a fairly long-term benefit that will require additional technical work to realize". Or did I miss something? Cheers Andreas -- ,,, (o o) +-ooO-(_)-Ooo-+ | Andreas Heigl | | mailto:andr...@heigl.org N 50°22'59.5" E 08°23'58" | | https://andreas.heigl.org | +-+ | https://hei.gl/appointmentwithandreas | +-+ OpenPGP_0xA8D5437ECE724FE5.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Tue, Nov 16, 2021 at 3:59 PM James Gilliland wrote: > On Tue, Nov 16, 2021 at 4:23 AM Rowan Tommins > wrote: > > > On 16/11/2021 09:27, Andreas Heigl wrote: > > > > > >> I see, yes, code that is 100% perfectly tested can get away without > > >> the language performing any error checking at all - the behaviour is > > >> all guaranteed by the tests. I would be very surprised if even 1% of > > >> PHP applications can claim such comprehensive tests. > > > > > > The topic here was that new code can verify the declaration of a > > > property by using tests. That does not need to happen on the language > > > level. I was never talking about adding tests for existing code. > > > > > > > > For most code bases, even new ones being written from scratch in PHP > > 8.0, that level of testing simply doesn't exist, and having the language > > tell you "hey, you wrote $this->loger instead of $this->logger" is a > > useful feature. And, in a lot of cases, more useful than having the > > language say "OK, I've created your dynamic $loger property for you", > > which is what currently happens. > > > > What you described there sounds like a warning and not a fatal error. Maybe > that's where some of the trepidation is coming from. I know I'm less > worried about the deprecation notice and more worried about what happens in > PHP 9 when it's a fatal error. > I can't say that this line of reasoning has its merits, but then there's no benefit to the engine itself. Issuing a warning and carry on materializing dynamic properties will never bring the original performance improvement that was part of the original state of the RFC. -- Marco Aurélio Deleu
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Tue, Nov 16, 2021 at 4:23 AM Rowan Tommins wrote: > On 16/11/2021 09:27, Andreas Heigl wrote: > > > >> I see, yes, code that is 100% perfectly tested can get away without > >> the language performing any error checking at all - the behaviour is > >> all guaranteed by the tests. I would be very surprised if even 1% of > >> PHP applications can claim such comprehensive tests. > > > > The topic here was that new code can verify the declaration of a > > property by using tests. That does not need to happen on the language > > level. I was never talking about adding tests for existing code. > > > > For most code bases, even new ones being written from scratch in PHP > 8.0, that level of testing simply doesn't exist, and having the language > tell you "hey, you wrote $this->loger instead of $this->logger" is a > useful feature. And, in a lot of cases, more useful than having the > language say "OK, I've created your dynamic $loger property for you", > which is what currently happens. > What you described there sounds like a warning and not a fatal error. Maybe that's where some of the trepidation is coming from. I know I'm less worried about the deprecation notice and more worried about what happens in PHP 9 when it's a fatal error.
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On 16/11/2021 09:27, Andreas Heigl wrote: I see, yes, code that is 100% perfectly tested can get away without the language performing any error checking at all - the behaviour is all guaranteed by the tests. I would be very surprised if even 1% of PHP applications can claim such comprehensive tests. The topic here was that new code can verify the declaration of a property by using tests. That does not need to happen on the language level. I was never talking about adding tests for existing code. Whether the code is "new" or "old" is not what matters; what matters is whether the test suite is comprehensive enough that every possible mistake will be caught by a test. If it will, we can remove a whole bunch of language features - why use parameter and property types, for instance, if your tests guarantee that they will always be given correct values? For most code bases, even new ones being written from scratch in PHP 8.0, that level of testing simply doesn't exist, and having the language tell you "hey, you wrote $this->loger instead of $this->logger" is a useful feature. And, in a lot of cases, more useful than having the language say "OK, I've created your dynamic $loger property for you", which is what currently happens. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Hi list, On Tue, 16 Nov 2021, 00:21 Deleu, wrote: > I see a pattern in these discussions from two mindsets: one thinking about > how we should design the future and another thinking about how we preserve > the past. I would frame that differently as those who 1. Predominantly write new code (most of us on this list I expect) 2. Use ecosystems written in PHP (talking from personal experience I have WordPress and Drupal in mind, but I see it with Laravel and to a smaller extent Symfony users too). Both approaches are massive audiences for PHP and both have different needs. For the second group, finding that something you built on no longer works because "PHP changed" is a problem, in a way it isn't for members of this list, who would likely fork and fix it. It's easy to use the glib response that "it's open source, they should get their hands dirty" but for many, knowing how to get started is a barrier that stops them. Making a big generalisation, this audience is hugely proud of what they do manage to accomplish and can be hugely vocal proponents for PHP (ecosystems). Yet it's not always lack of knowledge either that makes small backwards-incompatible changes a problem. If you're a small business your focus is elsewhere and what those working at larger and highly profitable or well funded companies see as a small expense (however annoying) is a big loss of other opportunities that a small business could use to grow. I see this with some clients, where a day is a big deal, and a day spent testing an upgrade is a day not spent fixing the problem 5% of customers have. I take it as read that we want to continue serving both audiences. On improving PHP, there is a prevalent view on this list that the competition is people going to other programming languages. The hidden competition is people saying "keeping up isn't worth the effort" and switching to centralised SaaS offerings. We see all the good reasons for doing this playing out in our own "Switch to GitHub Issues" discussion. It's happening enough, we don't need to encourage it. Returning to this RFC it may surprise people that I like the idea of only having dynamic properties on stdClass & children. I don't go as far as saying "this is how it should have been from the start" as the culture was different when classes were introduced to PHP. But dynamic properties have been around since the start and I am wary of deciding to remove them in PHP 9. Kind regards, Peter
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Hey all On 16.11.21 10:16, Rowan Tommins wrote: On 15/11/2021 17:20, Andreas Heigl wrote: When you are testing whether writing 'X' to a property and then reading from that property gives that 'X' back, then everything should be good. I see, yes, code that is 100% perfectly tested can get away without the language performing any error checking at all - the behaviour is all guaranteed by the tests. I would be very surprised if even 1% of PHP applications can claim such comprehensive tests. The topic here was that new code can verify the declaration of a property by using tests. That does not need to happen on the language level. I was never talking about adding tests for existing code. Yes: That rips off a completely different topic: Testing "getters" and "setters". Unless you have zero business logic in your classes, testing getters and setters is absolutely not sufficient. Everywhere that accesses a private or protected property has the potential to mistype it and cause subtle bugs. That is absolutely correct. The main question is: "Do we *need* to spot this behaviour in old code"? Not "Do we *want* to spot this behaviour in old code". Yes to both questions. A number of "uses" of this feature are not actually deliberate uses which need protecting by adding the attribute, they are mistakes that are going unnoticed in those warrens of untested, un-analysed code. Those code bases are exactly the ones that benefit from the language having the run-time checks built in. That highly depends on your view-point. There are a lot of people out there that do not require that this behaviour *needs* to be spotted. Cheers Andreas -- ,,, (o o) +-ooO-(_)-Ooo-+ | Andreas Heigl | | mailto:andr...@heigl.org N 50°22'59.5" E 08°23'58" | | https://andreas.heigl.org | +-+ | https://hei.gl/appointmentwithandreas | +-+ OpenPGP_0xA8D5437ECE724FE5.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On 15/11/2021 17:20, Andreas Heigl wrote: When you are testing whether writing 'X' to a property and then reading from that property gives that 'X' back, then everything should be good. I see, yes, code that is 100% perfectly tested can get away without the language performing any error checking at all - the behaviour is all guaranteed by the tests. I would be very surprised if even 1% of PHP applications can claim such comprehensive tests. Yes: That rips off a completely different topic: Testing "getters" and "setters". Unless you have zero business logic in your classes, testing getters and setters is absolutely not sufficient. Everywhere that accesses a private or protected property has the potential to mistype it and cause subtle bugs. That is absolutely correct. The main question is: "Do we *need* to spot this behaviour in old code"? Not "Do we *want* to spot this behaviour in old code". Yes to both questions. A number of "uses" of this feature are not actually deliberate uses which need protecting by adding the attribute, they are mistakes that are going unnoticed in those warrens of untested, un-analysed code. Those code bases are exactly the ones that benefit from the language having the run-time checks built in. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Tue, Nov 16, 2021, 00:36 wrote: > L.S., > > As some of you may have seen, I posted a thread on Twitter a few days > back referencing this RFC: > https://twitter.com/jrf_nl/status/1459221549429542920 > > I've been asked to post the link to the Twitter discussion in this > thread for visibility. > > The Twitter thread generated, and is still generating, quite a lot of > discussion, which I believe is a good thing and I'd like to thank > everyone of you who has been actively participating in these discussions > and has taken the time to read the various opinions. > > The thread, however, is not specifically about this RFC, but more about > the more poignant issue of the current pace of deprecations in PHP, the > impact of the workload these cause for open source project maintainers > and the stagnation I'm currently seeing in PHP open source releases and > innovation as a result of this. > > As I'm posting here now anyway, I'd like to leave the following feedback > for your consideration: > > From what I've gathered from responses and the added "Motivation" > section (thanks Nikita), this RFC is intended as a stepping stone > towards eventually removing support for dynamic properties from PHP > completely. > This only got cursory mention in the RFC and the RFC as it is, does not > lay out a clear path towards that end-goal. > If memory serves me right, Nikita planned to always leave dynamic properties bound to a single class (possibly stdClass as it is its natural use). That would have allowed for performance improvement on the engine. However, as we've seen from the Serializable RFC, planning out a 2-major change means voting today for a change that will only take place in 10 years. That's too long of a time to predict whether the change being voted even makes sense. As it currently stands, I believe the RFC outlines the best it can do with the context in which we are. If nothing else ever happens in the matter, we're left in a better place anyway. > What I miss in this RFC - and also oftentimes in other RFCs over the > past few years - are: > * The real reason behind this change proposal. "Modern code doesn't use > this" is not a reason and also not always true. > * An impact and workload analysis for userland PHP code, no matter how > tentative. > * Argumentation that this is the right stepping stone towards eventually > being able to achieve the end-goal of removing support altogether and a > tentative outline of what the path towards that end-goal will look like, > both in timeline, next steps and (tentative) criteria of when it would > be acceptable to take those steps. > I see a pattern in these discussions from two mindsets: one thinking about how we should design the future and another thinking about how we preserve the past. In the past static analyzes wasn't even a thing in PHP. Nowadays we produce code that is much more likely to be analyzable in the future than a decade or two ago. The camp of people that has to deal with code that "is better left untouched" will often find these type of RFC lacking in motivation i.e "why are you making me touch that code that I don't want to touch or shouldn't have to touch?". Likewise the people that has already suffered through these code and want to prevent them from being a constant problem will need little convincing that the change is good. > Without the above, the RFC as it currently is, is IMHO just creating > more busy-work without a clear path forward. > > Please also take note that while this deprecation may not have much of > an impact on applications which have full control over their server and > the PHP version on which the code is run, as those have a) full access > to server logs and b) can use tools like Rector to upgrade (once a > Rector for this has been written), this is a whole different ball-game > for open source. > > As pointed out before, static analysis tools (once written) can help, > but may struggle to analyse the code using dynamic properties correctly > in all cases. > > In the end, if this gets deprecated, the best way to find the potential > problematic instances, is, like always, a test suite with near full code > coverage to see all deprecations, but let's also be realistic: a full > test suite is a luxury few open source projects actually have. > > And even when a full test suite is available, and this is probably the > most important problem I see: **open source libraries are generally > building-blocks in a larger whole, where the library itself has no > control over how their users are using the code, let alone have any > indication of whether their users may be using or extending the library > in ways which _rely_ on dynamic properties.** > This uncertainty may lead to "abuse" of the attribute to prevent > introducing a breaking change. > I know you didn't mean it that way but I can't help but interpret this as a great motivation on why this RFC should be approved. It allows for a world to exist (in 3 years)
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Mon, Nov 15, 2021 at 3:11 PM Deleu wrote: > By design my REST API utilizes dynamic properties. I've always found it to >> be a feature of PHP, not a bug. >> >> -- >> Chase Peeler >> chasepee...@gmail.com >> > > I am in the unfortunate position of inheriting a system with such REST API > design. I can never build new REST APIs to replace the old ones because > nobody can ever know how many combinations of possible input parameters > there are. > > Our inputs and outputs are both well defined. I've built a framework around our entities that convert them into API payloads - attributes (symfony, but eventually we might update it to use native attributes) define what fields are visible based on user and whether it's the short (e.g. part of a collection) or full version. The thing is that occasionally we'll have a payload to return that requires additional attributes. Dynamic properties allow us to just tack that on to the existing entity without having to define it as a property on the entity (outside of the one use case the property isn't needed and it definitely doesn't correspond to a database column). > -- > Marco Aurélio Deleu > -- Chase Peeler chasepee...@gmail.com
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Den 2021-11-15 kl. 10:52, skrev Derick Rethans: Dear Internals, On Wed, 10 Nov 2021, Nikita Popov wrote: On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov wrote: This RFC takes the more direct route of deprecating this functionality entirely. I expect that this will have relatively little impact on modern code (e.g. in Symfony I could fix the vast majority of deprecation warnings with a three-line diff), but may have a big impact on legacy code that doesn't declare properties at all. I plan to open voting on this RFC soon. Most of the feedback was positive, apart from the initial choice of opt-int mechanism, and that part should be addressed by the switch to the #[AllowDynamicProperties] attribute. The voting is now open, but I think one thing was not taken into account here, the many small changes that push work to maintainers of Open Source library and CI related tools. In the last few years, the release cadence of PHP has increased, which is great for new features. It however has not been helpful to introduce many small deprecations and BC breaks in every single release. This invariably is making maintainers of Open Source anxious, and frustrated as so much work is need to keep things up to date. I know they are *deprecations*, and applications can turn these off, but that's not the case for maintainers of libraries. Before we introduce many more of this into PHP 8.2, I think it would be wise to figure out a way how to: - improve the langauge with new features - keep maintenance cost for open source library and CI tools much lower - come up with a set of guidelines for when it is necessary to introduce BC breaks and deprecations. I am all for improving the language and making it more feature rich, but we have not spend enough time considering the impacts to the full ecosystem. I have therefore voted "no" on this RFC, and I hope you will too. cheers, Derick Hi, Maybe the RM's should have a mandate to keep track on deprecations for a specific release and be able to say: "Sorry the shop are closed for further deprecations in this release". What do you think? One could count the deprecations in the 8.x track and have a straw poll on it and/or ask what key deprecations do we see further for the 8.x? Is even the "Dynamic properties" one, one of the last ones? r//Björn L -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
> > By design my REST API utilizes dynamic properties. I've always found it to > be a feature of PHP, not a bug. > > -- > Chase Peeler > chasepee...@gmail.com > I am in the unfortunate position of inheriting a system with such REST API design. I can never build new REST APIs to replace the old ones because nobody can ever know how many combinations of possible input parameters there are. -- Marco Aurélio Deleu
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Mon, Nov 15, 2021 at 3:02 PM James Gilliland wrote: > On Mon, Nov 15, 2021 at 3:53 AM Derick Rethans wrote: > > > Dear Internals, > > > > On Wed, 10 Nov 2021, Nikita Popov wrote: > > > > > On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov > > wrote: > > > > > > > This RFC takes the more direct route of deprecating this > > > > functionality entirely. I expect that this will have relatively > > > > little impact on modern code (e.g. in Symfony I could fix the vast > > > > majority of deprecation warnings with a three-line diff), but may > > > > have a big impact on legacy code that doesn't declare properties at > > > > all. > > > > > > > > > > I plan to open voting on this RFC soon. Most of the feedback was > > > positive, apart from the initial choice of opt-int mechanism, and that > > > part should be addressed by the switch to the > > > #[AllowDynamicProperties] attribute. > > > > The voting is now open, but I think one thing was not taken into account > > here, the many small changes that push work to maintainers of Open > > Source library and CI related tools. > > > > In the last few years, the release cadence of PHP has increased, which > > is great for new features. It however has not been helpful to introduce > > many small deprecations and BC breaks in every single release. > > > > This invariably is making maintainers of Open Source anxious, and > > frustrated as so much work is need to keep things up to date. I know > > they are *deprecations*, and applications can turn these off, but that's > > not the case for maintainers of libraries. > > > > Before we introduce many more of this into PHP 8.2, I think it would be > > wise to figure out a way how to: > > > > - improve the langauge with new features > > - keep maintenance cost for open source library and CI tools much lower > > - come up with a set of guidelines for when it is necessary to introduce > > BC breaks and deprecations. > > > > I am all for improving the language and making it more feature rich, but > > we have not spend enough time considering the impacts to the full > > ecosystem. > > > > I have therefore voted "no" on this RFC, and I hope you will too. > > > > cheers, > > Derick > > > > I appreciate that Derick. I know there have been some pretty big efforts > required for some recent php updates in Drupal. And I've mentioned in a > couple threads on Twitter but Drupal requires a pretty heavy change to > support this. It adds properties to classes _outside_ its codebase which > means none of the mitigations in the RFC apply. It was on my radar when the > vote popped up because the system in question has long been known to be > less than ideal but "php wouldn't ever remove this ancient feature right?" > and every other option is just a ton more complicated. I've talked with > some maintainers and it looks like we're going to deal with the cost of > converting the system but if this dirty hack hadn't been on our radar it > could have really hurt. Like maybe not getting the fix in place before the > next major release in time for backwards compatibility commitments bad. > > So I worry what sort of earthquake this might trigger through libraries. > Like, I'm pretty sure the OpenApiGenerator templates used dynamic > properties for some things so how many little internal libraries are going > to have to be regenerated? What other lightly maintained library are people > going to be stuck waiting to update because someone's CI didn't catch the > deprecation? > By design my REST API utilizes dynamic properties. I've always found it to be a feature of PHP, not a bug. > > I think this sort of change is probably for the better, but I worry about > how disruptive this could end up being. > -- Chase Peeler chasepee...@gmail.com
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Mon, Nov 15, 2021 at 3:53 AM Derick Rethans wrote: > Dear Internals, > > On Wed, 10 Nov 2021, Nikita Popov wrote: > > > On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov > wrote: > > > > > This RFC takes the more direct route of deprecating this > > > functionality entirely. I expect that this will have relatively > > > little impact on modern code (e.g. in Symfony I could fix the vast > > > majority of deprecation warnings with a three-line diff), but may > > > have a big impact on legacy code that doesn't declare properties at > > > all. > > > > > > > I plan to open voting on this RFC soon. Most of the feedback was > > positive, apart from the initial choice of opt-int mechanism, and that > > part should be addressed by the switch to the > > #[AllowDynamicProperties] attribute. > > The voting is now open, but I think one thing was not taken into account > here, the many small changes that push work to maintainers of Open > Source library and CI related tools. > > In the last few years, the release cadence of PHP has increased, which > is great for new features. It however has not been helpful to introduce > many small deprecations and BC breaks in every single release. > > This invariably is making maintainers of Open Source anxious, and > frustrated as so much work is need to keep things up to date. I know > they are *deprecations*, and applications can turn these off, but that's > not the case for maintainers of libraries. > > Before we introduce many more of this into PHP 8.2, I think it would be > wise to figure out a way how to: > > - improve the langauge with new features > - keep maintenance cost for open source library and CI tools much lower > - come up with a set of guidelines for when it is necessary to introduce > BC breaks and deprecations. > > I am all for improving the language and making it more feature rich, but > we have not spend enough time considering the impacts to the full > ecosystem. > > I have therefore voted "no" on this RFC, and I hope you will too. > > cheers, > Derick > I appreciate that Derick. I know there have been some pretty big efforts required for some recent php updates in Drupal. And I've mentioned in a couple threads on Twitter but Drupal requires a pretty heavy change to support this. It adds properties to classes _outside_ its codebase which means none of the mitigations in the RFC apply. It was on my radar when the vote popped up because the system in question has long been known to be less than ideal but "php wouldn't ever remove this ancient feature right?" and every other option is just a ton more complicated. I've talked with some maintainers and it looks like we're going to deal with the cost of converting the system but if this dirty hack hadn't been on our radar it could have really hurt. Like maybe not getting the fix in place before the next major release in time for backwards compatibility commitments bad. So I worry what sort of earthquake this might trigger through libraries. Like, I'm pretty sure the OpenApiGenerator templates used dynamic properties for some things so how many little internal libraries are going to have to be regenerated? What other lightly maintained library are people going to be stuck waiting to update because someone's CI didn't catch the deprecation? I think this sort of change is probably for the better, but I worry about how disruptive this could end up being.
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Mon, Nov 15, 2021, at 10:52 AM, Rowan Tommins wrote:
> On 15/11/2021 16:23, Andreas Heigl wrote:
>> One thing, that can verify the correct working of properties, whether
>> that is dynamic or static ones, is testing.
>
>
> Can it? I can't actually see how that would work, without also having a
> way to detect when dynamic properties were accessed, which brings us
> full circle. Also:
>
>
>> So the mistakes-part would be easy to handle.
>
> If you are working with a million lines of code going back 20 years,
> "write tests for everything" is not "easy to handle"; it's a long-term
> ambition which you chip away at when priorities allow.
>
> "Logging all deprecations and warnings on a production workload",
> however, *is* easy to handle. Diagnostic messages in logs are the *only*
> way this behaviour will be spotted in old code.
>
>
>> What I am still missing is the differentiation between "everything is
>> strict and you have to explicitly opt-in to make it dynamic" and an
>> "everything is dynamic and you can use a marker to mark this
>> explicitly an intended behaviour". That would allow users to mark a
>> class explicitly to use dynamic features even though it would make no
>> difference code-wise.
>
>
> I'm not sure what you mean by that. Do you mean, create the attribute,
> but don't actually do anything with it? Would the plan be to deprecate
> later? Never remove at all?
A possible idea to help make this transition (which I do support) more gradual:
Instead of an "allow" attribute, introduce a boolean flag attribute.
#[DynamicProperties(true)]
class Beep {}
The attribute marks whether dynamic properties are enabled or disabled via
boolean. If disabled, then they're an error if used.
8.2: Introduce the attribute, with a default of TRUE. Exactly zero code
breaks, but people can start adding the attribute now. That means people who
want to lock-out dynamic properties can do so starting now, and those cases
that do need them (or can't easily get away from them) can be flagged far in
advance.
8.something: Change the default to FALSE. Using dynamic properties when false
throws a deprecation, not an error. People have had some number of years to
add the attribute either direction if desired.
9.0: Change the deprecation to an error, if the attribute is set false (which
by now is the default) and dynamic properties are used.
Sometime in the future: Setting the attribute to true triggers a deprecation.
Sometime in the future: Remove dynamic properties entirely, and the attribute
along with it. People can use __get/__set if they really need.
This still gets us to the same place in the end, but introduces another stage
along the way to make the transition more gradual. We can debate which version
the default should flip in, I don't much mind. That could even wait for 9 if
we want to be really really gradual about it.
Meanwhile, IDEs and code templates can start including the attribute set to
false by default on any new classes that get written, just like they have done
for years with strict types.
Nikita, would that be feasible? Matthew, Derick, would that be satisfactory?
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Hey Rowan, hey all On 15.11.21 17:52, Rowan Tommins wrote: On 15/11/2021 16:23, Andreas Heigl wrote: One thing, that can verify the correct working of properties, whether that is dynamic or static ones, is testing. Can it? I can't actually see how that would work, without also having a way to detect when dynamic properties were accessed, which brings us full circle. Also: When you are testing whether writing 'X' to a property and then reading from that property gives that 'X' back, then everything should be good. You either typed the name of the property correctly or you have a typo in the setter and getter (or however you set and got the value). Nevertheless you should be able to figure out whether you accidentally misstyped a property name somewhere as that should break the test (unless you misstyped the name twice). ANd whether you are doing the assignement to a static property or to a dynamic one should be irrelevant, the reading and the writing process are the same. Yes: That rips off a completely different topic: Testing "getters" and "setters". So the mistakes-part would be easy to handle. If you are working with a million lines of code going back 20 years, "write tests for everything" is not "easy to handle"; it's a long-term ambition which you chip away at when priorities allow. The intention is not to write tests for existing code. As the intention is exactly to be able to leave existing code as it is. Otherwise the approach to "add the Attribute and be done" would be much easier. The intention is much more to make sure that *new* code does not write accidentally to the wrong property. Which is what this RFC is all about. Make sure that dynamic properties are not accidentally used. "Logging all deprecations and warnings on a production workload", however, *is* easy to handle. Diagnostic messages in logs are the *only* way this behaviour will be spotted in old code. That is absolutely correct. The main question is: "Do we *need* to spot this behaviour in old code"? Not "Do we *want* to spot this behaviour in old code". What I am still missing is the differentiation between "everything is strict and you have to explicitly opt-in to make it dynamic" and an "everything is dynamic and you can use a marker to mark this explicitly an intended behaviour". That would allow users to mark a class explicitly to use dynamic features even though it would make no difference code-wise. I'm not sure what you mean by that. Do you mean, create the attribute, but don't actually do anything with it? Would the plan be to deprecate later? Never remove at all? As currently there is no direct intention to remove the feature for good, why should we force it? And espechialy why do we need to force those maintinaing existing code to adapt their code? For now the possibility could be to keep everything *as is*. Plus add an attribute to make absolutely explicit that we want to use this feature. The next step could then be to create a setting that will notify about dynamic assignments in classes that are not marked by the attribute. I'm not talking about a deprecation note here. More something like a notice (not a warning as that is too severe!). That way the behaviour can come up in log files. Perhaps a new Level of notice like a "bad_coding_practice". Or we use different "lanes" of reporting. One for notices, errors, warnings et al and one for deprecations and such asignment-oddities. When that has been done (should so far all be BC) and code-maintainers have had enough time to modify their codebase (definition of "enough time" is here clearly the main topic and needs discussion with maintainers!) then we can talk about possibly phasing out the feature. My 0.02€ Cheers Andreas -- ,,, (o o) +-ooO-(_)-Ooo-+ | Andreas Heigl | | mailto:andr...@heigl.org N 50°22'59.5" E 08°23'58" | | https://andreas.heigl.org | +-+ | https://hei.gl/appointmentwithandreas | +-+ OpenPGP_0xA8D5437ECE724FE5.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On 15/11/2021 16:23, Andreas Heigl wrote: One thing, that can verify the correct working of properties, whether that is dynamic or static ones, is testing. Can it? I can't actually see how that would work, without also having a way to detect when dynamic properties were accessed, which brings us full circle. Also: So the mistakes-part would be easy to handle. If you are working with a million lines of code going back 20 years, "write tests for everything" is not "easy to handle"; it's a long-term ambition which you chip away at when priorities allow. "Logging all deprecations and warnings on a production workload", however, *is* easy to handle. Diagnostic messages in logs are the *only* way this behaviour will be spotted in old code. What I am still missing is the differentiation between "everything is strict and you have to explicitly opt-in to make it dynamic" and an "everything is dynamic and you can use a marker to mark this explicitly an intended behaviour". That would allow users to mark a class explicitly to use dynamic features even though it would make no difference code-wise. I'm not sure what you mean by that. Do you mean, create the attribute, but don't actually do anything with it? Would the plan be to deprecate later? Never remove at all? Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Hey Nikita. On 15.11.21 14:50, Nikita Popov wrote: [...] I hope that the new "Motivation" section clarifies things a bit, especially in regards to why "static analysis" is only a partial solution to this problem: https://wiki.php.net/rfc/deprecate_dynamic_properties#motivation Thanks for the clarification! One thing, that can verify the correct working of properties, whether that is dynamic or static ones, is testing. So when one wants to make sure that their code behaves as it should, then proper testing can help with that. So while the static analysis is one possibility, the other one is writing appropriate tests. While that will still not eliminate the usage of external tools it is fore sure something that most of the projects using modern code already implement. So the mistakes-part would be easy to handle. Being explicit on the other hand is something that I would very much appreciate. And using an Annotation for that is great. What I am still missing is the differentiation between "everything is strict and you have to explicitly opt-in to make it dynamic" and an "everything is dynamic and you can use a marker to mark this explicitly an intended behaviour". That would allow users to mark a class explicitly to use dynamic features even though it would make no difference code-wise. And that could already be implemented via a userland-shim right now which would then use the Core-Attribute as of PHP8.2 The other thing, that I noticed is that you were speaking of a possible reduction of the object-size by 8 byte. Is there a comparison of how much that would be in percent for some default applications? So is that something that actually "pays off"? Thanks again for your insights! Cheers Andreas -- ,,, (o o) +-ooO-(_)-Ooo-+ | Andreas Heigl | | mailto:andr...@heigl.org N 50°22'59.5" E 08°23'58" | | https://andreas.heigl.org | +-+ | https://hei.gl/appointmentwithandreas | +-+ OpenPGP_0xA8D5437ECE724FE5.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Mon, Nov 15, 2021 at 2:52 PM Andreas Heigl wrote: > > And as far as I can see from the PR associated with this RFC it will not > make life easier for the internals team. It is not like there will be > hundreds of lines code less to maintain. On the contrary. There is more > code and more logic to maintain [2]. > Sometimes it needs to be worse until it's better. Some points that evolved during discussion also mentioned the intention of how easy to allow it to be to opt-in and in the end the attribute was chosen as the easiest one. Even if the intention was to simplify the code to maintain, it was not clear how much PHP users would want to stay without this feature. And the problem was that using the attribute, it would not be easy to remove it in PHP 9. But at least it would give a better sense of usage once we get to PHP 9 so it can be completely removed only in PHP 10+ or to use a more strict opt-in mechanism. So yes, you are right, having it like this would make the code a bit worse to maintain for 8 years and easier to maintain after that, if I got it right. But the benefit related to the dynamic properties bugs reduction would be seen in userland starting with PHP 8.2. Regards, Alex
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Mon, Nov 15, 2021 at 1:52 PM Andreas Heigl wrote: > Hea all. > > On 15.11.21 10:52, Derick Rethans wrote: > > Dear Internals, > > > > On Wed, 10 Nov 2021, Nikita Popov wrote: > > > >> On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov > wrote: > >> > >>> This RFC takes the more direct route of deprecating this > >>> functionality entirely. I expect that this will have relatively > >>> little impact on modern code (e.g. in Symfony I could fix the vast > >>> majority of deprecation warnings with a three-line diff), but may > >>> have a big impact on legacy code that doesn't declare properties at > >>> all. > >>> > >> > >> I plan to open voting on this RFC soon. Most of the feedback was > >> positive, apart from the initial choice of opt-int mechanism, and that > >> part should be addressed by the switch to the > >> #[AllowDynamicProperties] attribute. > > > > The voting is now open, but I think one thing was not taken into account > > here, the many small changes that push work to maintainers of Open > > Source library and CI related tools. > > > > In the last few years, the release cadence of PHP has increased, which > > is great for new features. It however has not been helpful to introduce > > many small deprecations and BC breaks in every single release. > > > > This invariably is making maintainers of Open Source anxious, and > > frustrated as so much work is need to keep things up to date. I know > > they are *deprecations*, and applications can turn these off, but that's > > not the case for maintainers of libraries. > > > > Before we introduce many more of this into PHP 8.2, I think it would be > > wise to figure out a way how to: > > > > - improve the langauge with new features > > - keep maintenance cost for open source library and CI tools much lower > > - come up with a set of guidelines for when it is necessary to introduce > >BC breaks and deprecations. > > > > I am all for improving the language and making it more feature rich, but > > we have not spend enough time considering the impacts to the full > > ecosystem. > > > > I have therefore voted "no" on this RFC, and I hope you will too. > > > > cheers, > > Derick > > After some thoughs on this RFC I have reverted my original vote and > voted "No" due to several reasons. > > For one thing it is not clear to me what the benefits are. > That's my bad! The RFC did not really go into the motivation for the change, especially after I dropped the discussion of internal motivations in a later revision. I hope that the new "Motivation" section clarifies things a bit, especially in regards to why "static analysis" is only a partial solution to this problem: https://wiki.php.net/rfc/deprecate_dynamic_properties#motivation Regards, Nikita
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Mon, Nov 15, 2021 at 1:52 PM Andreas Heigl wrote:
> Hea all.
>
> On 15.11.21 10:52, Derick Rethans wrote:
> > Dear Internals,
> >
> > On Wed, 10 Nov 2021, Nikita Popov wrote:
> >
> >> On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov
> wrote:
> >>
> >>> This RFC takes the more direct route of deprecating this
> >>> functionality entirely. I expect that this will have relatively
> >>> little impact on modern code (e.g. in Symfony I could fix the vast
> >>> majority of deprecation warnings with a three-line diff), but may
> >>> have a big impact on legacy code that doesn't declare properties at
> >>> all.
> >>>
> >>
> >> I plan to open voting on this RFC soon. Most of the feedback was
> >> positive, apart from the initial choice of opt-int mechanism, and that
> >> part should be addressed by the switch to the
> >> #[AllowDynamicProperties] attribute.
> >
> > The voting is now open, but I think one thing was not taken into account
> > here, the many small changes that push work to maintainers of Open
> > Source library and CI related tools.
> >
> > In the last few years, the release cadence of PHP has increased, which
> > is great for new features. It however has not been helpful to introduce
> > many small deprecations and BC breaks in every single release.
> >
> > This invariably is making maintainers of Open Source anxious, and
> > frustrated as so much work is need to keep things up to date. I know
> > they are *deprecations*, and applications can turn these off, but that's
> > not the case for maintainers of libraries.
> >
> > Before we introduce many more of this into PHP 8.2, I think it would be
> > wise to figure out a way how to:
> >
> > - improve the langauge with new features
> > - keep maintenance cost for open source library and CI tools much lower
> > - come up with a set of guidelines for when it is necessary to introduce
> >BC breaks and deprecations.
> >
> > I am all for improving the language and making it more feature rich, but
> > we have not spend enough time considering the impacts to the full
> > ecosystem.
> >
> > I have therefore voted "no" on this RFC, and I hope you will too.
> >
> > cheers,
> > Derick
>
> After some thoughs on this RFC I have reverted my original vote and
> voted "No" due to several reasons.
>
> For one thing it is not clear to me what the benefits are. Yes: The
> language evolution RFC talks about "Forbidding dynamic object
> properties" but it also specifies that "there is also a lot of old code
> that does not declare properties, so this needs to be opt-in"[1].
>
> And as far as I can see from the PR associated with this RFC it will not
> make life easier for the internals team. It is not like there will be
> hundreds of lines code less to maintain. On the contrary. There is more
> code and more logic to maintain [2].
>
This RFCs goal is not to have less code to maintain, but to fix a nasty
class of errors in user errors where they accidently write/read to a
dynamic property due to a typo, instead of accessing the declared one.
True this is a mistake of the RFC not to highlight more.
> So when the only reason for the change is that one line in the RFC ("In
> modern code, this is rarely done intentionally"[3]) then that is not
> enough of a reasoning for me for such a code change that requires a lot
> of existing code to change.
>
> Those that want a cleaner code can already use static code analysis to
> find such issues (if not, I'm sure that there will be some analyzers
> around before PHP8.2 will be around) or write appropriate tests to make
> sure that they do not use undeclared properties.
>
Code that intentionally or unintentionally uses dynamic properties often
does not write each propery explicitly:
$object->$columnName = $value;
This cannot be detected by static analysers.
For the case where you explitly write a property name, While static
analysis and IDEs do help detecing these as problems, this class of bugs
happens because you are *not* using an IDE but a text editor like
Vim/Notepad++ where you maybe add a typo to a property name while writing
code.
> While I personally would really like to deprecate dynamic properties I
> believe that it is the wrong thing to do for the language. At least
> given the presented arguments why we should do it.
>
> Cheers
>
> Andreas
>
> PS: Am I the only one missing whether this is a 2/3 or a 50%+1 vote in
> the RFC?
>
>
>
> [1]
>
> https://github.com/nikic/php-rfcs/blob/language-evolution/rfcs/-language-evolution.md#forbidding-dynamic-object-properties
> [2] https://github.com/php/php-src/pull/7571/files
> [3] https://wiki.php.net/rfc/deprecate_dynamic_properties
>
> --
>,,,
> (o o)
> +-ooO-(_)-Ooo-+
> | Andreas Heigl |
> | mailto:andr...@heigl.org
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On 15.11.21 14:19, Rowan Tommins wrote: On 15/11/2021 12:51, Andreas Heigl wrote: PS: Am I the only one missing whether this is a 2/3 or a 50%+1 vote in the RFC? All votes require a 2/3 majority as of https://wiki.php.net/rfc/abolish-narrow-margins Thanks! I just stumbled over the fact that some other recent RFCs still had the statement "requiring a 2/3 majority" in the "Vote" part which I was missing here. Cheers Andreas -- ,,, (o o) +-ooO-(_)-Ooo-+ | Andreas Heigl | | mailto:andr...@heigl.org N 50°22'59.5" E 08°23'58" | | https://andreas.heigl.org | +-+ | https://hei.gl/appointmentwithandreas | +-+ OpenPGP_0xA8D5437ECE724FE5.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On 15/11/2021 12:51, Andreas Heigl wrote: PS: Am I the only one missing whether this is a 2/3 or a 50%+1 vote in the RFC? All votes require a 2/3 majority as of https://wiki.php.net/rfc/abolish-narrow-margins -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Hea all.
On 15.11.21 10:52, Derick Rethans wrote:
Dear Internals,
On Wed, 10 Nov 2021, Nikita Popov wrote:
On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov wrote:
This RFC takes the more direct route of deprecating this
functionality entirely. I expect that this will have relatively
little impact on modern code (e.g. in Symfony I could fix the vast
majority of deprecation warnings with a three-line diff), but may
have a big impact on legacy code that doesn't declare properties at
all.
I plan to open voting on this RFC soon. Most of the feedback was
positive, apart from the initial choice of opt-int mechanism, and that
part should be addressed by the switch to the
#[AllowDynamicProperties] attribute.
The voting is now open, but I think one thing was not taken into account
here, the many small changes that push work to maintainers of Open
Source library and CI related tools.
In the last few years, the release cadence of PHP has increased, which
is great for new features. It however has not been helpful to introduce
many small deprecations and BC breaks in every single release.
This invariably is making maintainers of Open Source anxious, and
frustrated as so much work is need to keep things up to date. I know
they are *deprecations*, and applications can turn these off, but that's
not the case for maintainers of libraries.
Before we introduce many more of this into PHP 8.2, I think it would be
wise to figure out a way how to:
- improve the langauge with new features
- keep maintenance cost for open source library and CI tools much lower
- come up with a set of guidelines for when it is necessary to introduce
BC breaks and deprecations.
I am all for improving the language and making it more feature rich, but
we have not spend enough time considering the impacts to the full
ecosystem.
I have therefore voted "no" on this RFC, and I hope you will too.
cheers,
Derick
After some thoughs on this RFC I have reverted my original vote and
voted "No" due to several reasons.
For one thing it is not clear to me what the benefits are. Yes: The
language evolution RFC talks about "Forbidding dynamic object
properties" but it also specifies that "there is also a lot of old code
that does not declare properties, so this needs to be opt-in"[1].
And as far as I can see from the PR associated with this RFC it will not
make life easier for the internals team. It is not like there will be
hundreds of lines code less to maintain. On the contrary. There is more
code and more logic to maintain [2].
So when the only reason for the change is that one line in the RFC ("In
modern code, this is rarely done intentionally"[3]) then that is not
enough of a reasoning for me for such a code change that requires a lot
of existing code to change.
Those that want a cleaner code can already use static code analysis to
find such issues (if not, I'm sure that there will be some analyzers
around before PHP8.2 will be around) or write appropriate tests to make
sure that they do not use undeclared properties.
While I personally would really like to deprecate dynamic properties I
believe that it is the wrong thing to do for the language. At least
given the presented arguments why we should do it.
Cheers
Andreas
PS: Am I the only one missing whether this is a 2/3 or a 50%+1 vote in
the RFC?
[1]
https://github.com/nikic/php-rfcs/blob/language-evolution/rfcs/-language-evolution.md#forbidding-dynamic-object-properties
[2] https://github.com/php/php-src/pull/7571/files
[3] https://wiki.php.net/rfc/deprecate_dynamic_properties
--
,,,
(o o)
+-ooO-(_)-Ooo-+
| Andreas Heigl |
| mailto:andr...@heigl.org N 50°22'59.5" E 08°23'58" |
| https://andreas.heigl.org |
+-+
| https://hei.gl/appointmentwithandreas |
+-+
OpenPGP_0xA8D5437ECE724FE5.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Dear Internals, On Wed, 10 Nov 2021, Nikita Popov wrote: > On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov wrote: > > > This RFC takes the more direct route of deprecating this > > functionality entirely. I expect that this will have relatively > > little impact on modern code (e.g. in Symfony I could fix the vast > > majority of deprecation warnings with a three-line diff), but may > > have a big impact on legacy code that doesn't declare properties at > > all. > > > > I plan to open voting on this RFC soon. Most of the feedback was > positive, apart from the initial choice of opt-int mechanism, and that > part should be addressed by the switch to the > #[AllowDynamicProperties] attribute. The voting is now open, but I think one thing was not taken into account here, the many small changes that push work to maintainers of Open Source library and CI related tools. In the last few years, the release cadence of PHP has increased, which is great for new features. It however has not been helpful to introduce many small deprecations and BC breaks in every single release. This invariably is making maintainers of Open Source anxious, and frustrated as so much work is need to keep things up to date. I know they are *deprecations*, and applications can turn these off, but that's not the case for maintainers of libraries. Before we introduce many more of this into PHP 8.2, I think it would be wise to figure out a way how to: - improve the langauge with new features - keep maintenance cost for open source library and CI tools much lower - come up with a set of guidelines for when it is necessary to introduce BC breaks and deprecations. I am all for improving the language and making it more feature rich, but we have not spend enough time considering the impacts to the full ecosystem. I have therefore voted "no" on this RFC, and I hope you will too. cheers, Derick -- PHP 7.4 Release Manager Host of PHP Internals News: https://phpinternals.news Like Xdebug? Consider supporting me: https://xdebug.org/support https://derickrethans.nl | https://xdebug.org | https://dram.io twitter: @derickr and @xdebug -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Le 13/10/2021 à 14:42, Rowan Tommins a écrit :
On 13/10/2021 10:17, Guilliam Xavier wrote:
Off-topic:
if ( class_exists('\\WeakMap') ) {
People should stop using a leading slash in FQCN *strings*.
\Foo::class is "Foo", not "\Foo". It might work generally but that's
asking for problems (e.g. for autoloaders).
(Sorry.)
Hah, good point. I should probably just have just used ::class anyway
(in which case the leading backslash is the quickest way of the sample
code being pasteable into some other file):
if ( class_exists(\WeakMap::class) ) {
There's probably a bunch of other style no-noes in that example, but I
did at least test it worked. :)
Regards,
I am surprised that:
if ( class_exists(\WeakMap::class) ) {
Actually works when the class does not exist.
It's not really surprising, I mean the FQDN::class in the end only
resolve to a simple string no matter that the class exists or not, but,
I'm being the devil's advocate here, there are use case were you would
want the engine to crash when you explicitly reference/use a non
existing class, even when it's only its name.
I'm sorry this is off-topic.
--
Pierre
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On 13/10/2021 10:17, Guilliam Xavier wrote:
Off-topic:
if ( class_exists('\\WeakMap') ) {
People should stop using a leading slash in FQCN *strings*.
\Foo::class is "Foo", not "\Foo". It might work generally but that's
asking for problems (e.g. for autoloaders).
(Sorry.)
Hah, good point. I should probably just have just used ::class anyway
(in which case the leading backslash is the quickest way of the sample
code being pasteable into some other file):
if ( class_exists(\WeakMap::class) ) {
There's probably a bunch of other style no-noes in that example, but I
did at least test it worked. :)
Regards,
--
Rowan Tommins
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Le 13/10/2021 à 11:17, Guilliam Xavier a écrit :
Off-topic:
if ( class_exists('\\WeakMap') ) {
People should stop using a leading slash in FQCN *strings*. \Foo::class is
"Foo", not "\Foo". It might work generally but that's asking for problems
(e.g. for autoloaders).
(Sorry.)
And why not ? using the FQDN is 100% valid, autoloaders should take care
of this, not the end user.
For what it worth, I'm using both, depending upon context, both can make
sense.
Regards,
--
Pierre
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Wed, Oct 13, 2021 at 11:17 AM Guilliam Xavier
wrote:
> Off-topic:
>
> if ( class_exists('\\WeakMap') ) {
>>
>
> People should stop using a leading slash in FQCN *strings*. \Foo::class
> is "Foo", not "\Foo". It might work generally but that's asking for
> problems (e.g. for autoloaders).
>
> (Sorry.)
>
And of course I mistyped "leading backslash"... Also externals.io seems to
strip them, so: \\Foo::class is "Foo", not "\\Foo".
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
Off-topic:
if ( class_exists('\\WeakMap') ) {
>
People should stop using a leading slash in FQCN *strings*. \Foo::class is
"Foo", not "\Foo". It might work generally but that's asking for problems
(e.g. for autoloaders).
(Sorry.)
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On 13/10/2021 02:43, Tim Starling wrote:
I think it would still be the biggest compatibility break since PHP
4->5.
I think this is a rather large exaggeration. It's certainly a use case
we need to consider, but it's not on the scale of call-time
pass-by-reference, or removing the mysql extension, or a dozen other
changes that have happened over the years.
As the RFC notes, you could migrate to WeakMap, but it will be very
difficult to write code that works on both 7.x and 8.2, since both
attributes and WeakMap were only introduced in 8.0.
Firstly, writing code that works on both 7.x and 8.2 will be easy:
ignore the deprecation notices. As discussed elsewhere, treating a
deprecation as a breaking change makes a nonsense of deprecating anything.
Secondly, I would be surprised if libraries using this trick don't
already have helper functions for doing it, in which case changing those
to use WeakMap and falling back to the dynamic property implementation
on old versions is easy. Maybe I'm missing some difficulty here?
if ( class_exists('\\WeakMap') ) {
class Attacher {
private static $map;
public static function setAttachment(object $object, string
$name, $value): void
{
self::$map ??= new WeakMap;
$attachments = self::$map[$object] ?? [];
$attachments[$name] = $value;
self::$map[$object] = $attachments;
}
public static function getAttachment(object $object, string $name)
{
self::$map ??= new WeakMap;
return self::$map[$object][$name] ?? null;
}
}
}
else {
class Attacher {
public static function setAttachment(object $object, string
$name, $value): void
{
$attachments = $object->__attached_values__ ?? [];
$attachments[$name] = $value;
$object->__attached_values__ = $attachments;
}
public static function getAttachment(object $object, string $name)
{
return $object->__attached_values__[$name] ?? null;
}
}
}
$foo = new Foo;
Attacher::setAttachment($foo, 'hello', 'world');
echo Attacher::getAttachment($foo, 'hello'), "\n";
I would support an opt-in mechanism, although I think 8.2 is too soon
for all core classes to opt in to it. We already have classes that
throw an exception in __set(), so it would be nice to have some
syntactic sugar for that.
I proposed "locked classes" a while ago, but consensus was that this was
the wrong way around.
One suggestion that did come up there was a block-scopable declare which
would allow manipulating dynamic properties, even without the target
class's co-operation. However, I think WeakMap (with a helper and a
fallback like above) is probably a superior solution for that, because
adding properties outside the class always carries risk of name
collision, interaction with __get, etc
Regards,
--
Rowan Tommins
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Wed, Oct 13, 2021 at 3:43 AM Tim Starling wrote: > On 12/10/21 9:23 pm, Nikita Popov wrote: > > Based on the received feedback, I've updated the RFC to instead provide > an > > #[AllowDynamicProperties] attribute as a way to opt-in to the use of > > dynamic properties. As previously discussed, this won't allow us to > > completely remove dynamic properties from the language model anymore, but > > it will make the upgrade path smoother by avoiding multiple inheritance > > issues. Especially given recent feedback on backwards-incompatible > changes, > > I think it's prudent to go with the more conservative approach. > > I think it would still be the biggest compatibility break since PHP > 4->5. Adding a custom property is a common way for an extension to > attach data to an object generated by an uncooperative application or > library. > > As the RFC notes, you could migrate to WeakMap, but it will be very > difficult to write code that works on both 7.x and 8.2, since both > attributes and WeakMap were only introduced in 8.0. In MediaWiki > pingback data for the month of September, only 5.2% of users are on > PHP 8.0 or later. > Just to be clear on this point: While attributes have only been introduced in 8.0, they can still be used in earlier versions and are simply ignored there. It is safe to use #[AllowDynamicProperties] without version compatibility considerations. > Also, in the last week, I've been analyzing memory usage of our > application. I've come to a new appreciation of the compactness of > undeclared properties on classes with sparse data. You can reduce > memory usage by removing the declaration of any property that is null > on more than about 75% of instances. CPU time may also benefit due to > improved L2 cache hit ratio and reduced allocator overhead. > Huh, that's an interesting problem. Usually I only see the reverse situation, where accidentally materializing the dynamic property table (through foreach, array casts, etc) causes issues, because it uses much more memory than declared properties. Based on a quick calculation, the cost of having dynamic properties clocks in at 24 declared properties (376 bytes for the smallest non-empty HT vs 16 bytes per declared property), so that seems like it would usually be a bad trade off unless you already end up materializing dynamic properties for other reasons. Did you make sure that you do not materialize dynamic properties (already before un-declaring some properties)? > So if the point of the RFC is to eventually get rid of property > hashtables from the engine, I'm not sure if that would actually be a > win for performance. I'm more thinking about the need for a sparse > attribute which moves declared properties out of properties_table. > The ability to opt-in to dynamic properties would always remain in some form (if only through stdClass extension as originally proposed), so if you have a case where it makes sense, the option would still be there. Regards, Nikita
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On 12/10/21 9:23 pm, Nikita Popov wrote: > Based on the received feedback, I've updated the RFC to instead provide an > #[AllowDynamicProperties] attribute as a way to opt-in to the use of > dynamic properties. As previously discussed, this won't allow us to > completely remove dynamic properties from the language model anymore, but > it will make the upgrade path smoother by avoiding multiple inheritance > issues. Especially given recent feedback on backwards-incompatible changes, > I think it's prudent to go with the more conservative approach. I think it would still be the biggest compatibility break since PHP 4->5. Adding a custom property is a common way for an extension to attach data to an object generated by an uncooperative application or library. As the RFC notes, you could migrate to WeakMap, but it will be very difficult to write code that works on both 7.x and 8.2, since both attributes and WeakMap were only introduced in 8.0. In MediaWiki pingback data for the month of September, only 5.2% of users are on PHP 8.0 or later. Also, in the last week, I've been analyzing memory usage of our application. I've come to a new appreciation of the compactness of undeclared properties on classes with sparse data. You can reduce memory usage by removing the declaration of any property that is null on more than about 75% of instances. CPU time may also benefit due to improved L2 cache hit ratio and reduced allocator overhead. So if the point of the RFC is to eventually get rid of property hashtables from the engine, I'm not sure if that would actually be a win for performance. I'm more thinking about the need for a sparse attribute which moves declared properties out of properties_table. I would support an opt-in mechanism, although I think 8.2 is too soon for all core classes to opt in to it. We already have classes that throw an exception in __set(), so it would be nice to have some syntactic sugar for that. -- Tim Starling -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
On Tue, Oct 12, 2021 at 3:52 PM Levi Morrison wrote: > > Based on the received feedback, I've updated the RFC to instead provide > an > > #[AllowDynamicProperties] attribute as a way to opt-in to the use of > > dynamic properties. As previously discussed, this won't allow us to > > completely remove dynamic properties from the language model anymore, but > > it will make the upgrade path smoother by avoiding multiple inheritance > > issues. > > Quoting the updated RFC: > > We may still wish to remove dynamic properties entirely at some later > > point. Having the #[AllowDynamicProperties] attribute will make it much > > easier to evaluate such a move, as it will be easier to analyze how much > > and in what way dynamic properties are used in the ecosystem. > > But in this place it does not mention PHP 9.0. Other places still > mention converting it to an error for PHP 9.0. Is that still the plan? > The current RFC proposes to make dynamic properties an error for classes without #[AllowDynamicProperties] in PHP 9.0. On the other hand, classes using the attribute will be able to continue using dynamic properties without error. (That is, as usual: Deprecations are converted to error in the next major version.) Regards, Nikita
Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
> Based on the received feedback, I've updated the RFC to instead provide an > #[AllowDynamicProperties] attribute as a way to opt-in to the use of > dynamic properties. As previously discussed, this won't allow us to > completely remove dynamic properties from the language model anymore, but > it will make the upgrade path smoother by avoiding multiple inheritance > issues. Quoting the updated RFC: > We may still wish to remove dynamic properties entirely at some later > point. Having the #[AllowDynamicProperties] attribute will make it much > easier to evaluate such a move, as it will be easier to analyze how much > and in what way dynamic properties are used in the ecosystem. But in this place it does not mention PHP 9.0. Other places still mention converting it to an error for PHP 9.0. Is that still the plan? -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
