Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116335
User-Agent: Cyrus-JMAP/3.5.0-alpha0-1371-g2296cc3491-fm-20211109.003-g2296cc34
Mime-Version: 1.0
Message-ID: <f6c83821-e3f1-422b-914b-a26c7c64b934@www.fastmail.com>
In-Reply-To: 
 <CAJp_myU5Wc3heD0vB7P8a65x4zd2pBmh9jt8Jb38SP80Kz0JSQ@mail.gmail.com>
References: 
 <CAF+90c_wDsjuMFAoQLehkn5GeoLv5GByczF+o-eDmYe+VVRxTA@mail.gmail.com>
 <CAJp_myU5Wc3heD0vB7P8a65x4zd2pBmh9jt8Jb38SP80Kz0JSQ@mail.gmail.com>
Date: Fri, 12 Nov 2021 15:41:25 -0600
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain
Subject: Re: [PHP-DEV] [VOTE] Deprecate dynamic properties
From: larry@garfieldtech.com ("Larry Garfield")

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.

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.

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.)

--Larry Garfield