Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116336
MIME-Version: 1.0
References: <CAF+90c_wDsjuMFAoQLehkn5GeoLv5GByczF+o-eDmYe+VVRxTA@mail.gmail.com>
 <CAJp_myU5Wc3heD0vB7P8a65x4zd2pBmh9jt8Jb38SP80Kz0JSQ@mail.gmail.com> <f6c83821-e3f1-422b-914b-a26c7c64b934@www.fastmail.com>
In-Reply-To: <f6c83821-e3f1-422b-914b-a26c7c64b934@www.fastmail.com>
Date: Fri, 12 Nov 2021 16:34:01 -0600
Message-ID: <CAJp_myVPsGY023o6+qMYu95M8fBdtGMzyh7xf_NJZhisKZc7bA@mail.gmail.com>
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000000838cb05d09f0f01"
Subject: Re: [PHP-DEV] [VOTE] Deprecate dynamic properties
From: mweierophinney@gmail.com ("Matthew Weier O'Phinney")

--0000000000000838cb05d09f0f01
Content-Type: text/plain; charset="UTF-8"

On Fri, Nov 12, 2021, 3:41 PM Larry Garfield <larry@garfieldtech.com> wrote:

> On Fri, Nov 12, 2021, at 12:59 PM, Matthew Weier O'Phinney wrote:
> > I recognize it's a bit late to be commenting, now that voting has
> > started... but this feels like a solution for which we already have
> > workable solutions, and which will instead lead to a lot of
> > misunderstanding and breakage in the user ecosystem.
> >
> > Our IDEs, coding standards, and static analysis tools can already flag
> > these things for us, helping us catch them early. Hell, unit testing will
> > find these for us, when a test fails due to a value not being set in a
> > property that we expected.
> >
> > Making this fundamental change to the language means, however, that a lot
> > of things that we were previously able to do that "just worked" now
> raise a
> > deprecation notice, and, later, a compilation error... unless we make a
> > change to our already working, fully functional code.
> >
> > I think the Locked Classes approach made far more sense here, because it
> > didn't require changes to _working_ code. By making the behavior opt-in,
> > developers get to choose if that's what they want for their classes,
> > instead of having a backwards breaking change thrust on them.
> >
> > (Perhaps an even better solution would be a declaration, like we have for
> > strict_types, which we could do per file.)
> >
> > Or maybe I'm missing something else: was there something at the engine
> > level that was driving this, a significant performance gain we get from
> > changing the behavior? Because if there was, there was no discussion of
> it.
> > In fact, the only discussion of "why" in the RFC is "In modern code, this
> > is rarely done intentionally". Why is that justification for changing the
> > behavior? Can you quantify how much code would be affected by this
> change?
> > and how much would benefit?
> >
> > Yes, I know that code I write would benefit from it, and personally I'd
> > love to have a way to opt-in to something more strict - but I think a
> > switch like this is going to make upgrading to version 9 difficult if not
> > impossible for a huge number of PHP users.
> >
> > Sweeping changes like this need data behind them. If there's such data
> for
> > this RFC, it's not present in the proposal, and as such, I cannot
> > understand what drives it.
>
> The original version of the RFC would have (as of v9) allowed for the
> removal of some fugly code in property handling, resulting in engine
> improvements and some minor performance benefits.  That was because it
> pushed the opt-in mechanism to "only if you extend stdclass", and stdclass
> would effectively have a trait with stock __get/__set implementations to
> handle dynamic property behavior.  (It wasn't quite that, but effectively
> that's what it would do.)  So that was the original impetus.
>

None of that information was in the RFC. That sort of thing really needs to
be included, as it helps a ton in justifying changes of this nature. As I
noted, on reading it, the only rationale made is a quick mention that they
lead to programming errors, and even then, there's no detail really if
_how_.


> The current version doesn't actually remove the behavior yet, so it
> wouldn't allow for that optimization.  That does, I agree, weaken its
> argument, but as the RFC notes makes it easier in the future to figure out
> how widespread dynamic properties actually are in code today.  Right now...
> we simply don't know.  It's possible the impact here will be almost nil, or
> it could break anything written before 2010.  (I'd guess it's closer to the
> former than the latter, but no one knows for certain.)  Just grepping for
> the attribute in the future would give us a better picture of how common it
> is.
>

That's the thing: it's guesswork. About a fundamental way the object model
works. While most library code may not make use of it, I can think of a
number of ways I've seen it used or used the feature myself without even
trying too hard:

- In unit tests, to create quick spies to pass into closures.
- In API wrappers, to represent payloads of specific types. (This is
particularly interesting, as it allows for these wrappers to be forward
compatible with additions to the API payload).
- As internal DTO messages, particularly when prototyping. I get the
benefits of a typed object without needing to fully define its properties.

And that's just off the top of my head.


> To those that are arguing that the timeline is too aggressive, I would
> turn the question around: What timeline would be acceptable?  "Well
> behaved" code hasn't used dynamic properties outside of a few very edge
> bits in a very, very long time, so transitioning to those being the
> exception and not the default is, IMO, reasonable.  What would be a more
> comfortable timeline if not what is proposed here?


> (Bearing in mind the previous statements that treating deprecations as
> though they were warnings, even in production, is not Internals problem but
> a problem of people who have their servers misconfigured.)
>

I'm bringing this up again: because deprecations are reported by changing a
flag to error reporting, and because the default dev configuration enables
that flag, and it's included as part of E_ALL, the reality is that many
users see the deprecations.

When those errors point to _libraries_ they consume, they report them.

Yes, library maintainers can have policies to consider errors due to
deprecations as invalid. But they still need to evaluate and review those
issues, which takes time and effort. And ignoring the issue often means it
gets reported repeatedly.

The other option is to deal with the deprecation. But this then means
yearly churn, as the project creates new deprecations with each minor
version.

Listen, I'm happy about a lot of the changes to the language as a user. But
as a library maintainer:

- Changes to fundamental, decades long behavior has huge knock-on effects
for us, and causes a lot of busy work.

- Yearly deprecations trigger an onslought of issues and patches that do
nothing for the average user, and detract from other work, such as bug
fixes and feature additions.

I'd like for a discussion on how we can allow the language to develop
without requiring yearly churn and burden on the surrounding ecosystem. And
just waving your hands and saying deprecations aren't errors is not an
answer - if it makes its way to an error log or is reported inline in a
page or in CLI output, it's an error.


> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>

--0000000000000838cb05d09f0f01--