Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115820
User-Agent: Cyrus-JMAP/3.5.0-alpha0-1125-g685cec594c-fm-20210825.001-g685cec59
Mime-Version: 1.0
Message-ID: <2c6adf5e-3ebb-4179-b654-de91f33c06f7@www.fastmail.com>
In-Reply-To: 
 <CAESVnVqMfxSZxG1+k+_o+HZ1B7QbH=wDeMTQm8R-Ok_bEyqSRA@mail.gmail.com>
References: 
 <CAF+90c9_qu=PiTS81J2mesPfJdz7n5nSi3qoLNLjhEGN-aTytA@mail.gmail.com>
 <CAESVnVqMfxSZxG1+k+_o+HZ1B7QbH=wDeMTQm8R-Ok_bEyqSRA@mail.gmail.com>
Date: Wed, 25 Aug 2021 11:05:05 -0500
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [RFC] Deprecate dynamic properties
From: larry@garfieldtech.com ("Larry Garfield")

On Wed, Aug 25, 2021, at 10:28 AM, Sara Golemon wrote:
> On Wed, Aug 25, 2021 at 5:03 AM Nikita Popov <nikita.ppv@gmail.com> wr=
ote:
> > I'd like to propose the deprecation of "dynamic properties", that is
> > properties that have not been declared in the class (stdClass and
> > __get/__set excluded, of course):
> >
> > https://wiki.php.net/rfc/deprecate_dynamic_properties
> >
>=20
> > This RFC offers `extends stdClass` as a way to opt-in to the use of
> dynamic properties.
> >
> This makes the assumption that existing uses of dynamic properties are=
 all
> root classes. I don't think that assumption can be made.
>=20
> > Some people have suggested that we could use a magic marker
> > interface (implements SupportsDynamicProperties),
> > an attribute (#[SupportsDynamicProperties])
> > or a trait (use DynamicProperties;) instead.
> >
> My gut-check says an Attribute works well enough.  This will unlock the
> class (disable deprecation warning) in 8.2+ and be ignored in earlier
> releases (8.0 and 8.1 would need an auto-loaded polyfill userspace
> attribute obviously, but that's a tractable problem).
>=20
> #[SupportsDynamicProperties]
> class Foo { ... }
>=20
> > The Locked classes RFC took an alternative approach to this problem =
space:
> > Rather than deprecating/removing dynamic properties and providing an
> opt-in
> > for specific classes, it instead allowed marking specific classes as
> locked in
> > order to forbid creation of dynamic properties on them.
> >
> > I don't believe that this is the right strategy, because in contempo=
rary
> code,
> > classes being =E2=80=9Clocked=E2=80=9D is the default state, while c=
lasses that require
> dynamic
> > properties are a rare exception. Additionally, this requires that cl=
ass
> owners
> > (which may be 3rd party packages) consistently add the =E2=80=9Clock=
ed=E2=80=9D keyword
> > to be effective.
> >
> I struggle here, because the opt-in approach is the most BC, but I act=
ually
> think you're right.  I think[citation needed] that dynamic props are
> probably rare enough that as long as the escape hatch is clear, we can=
 be a
> little more bold about nudging the ecosystem forward.  I will counter
> however, that the same issue with 3rd party libraries applies to opt-o=
ut as
> opt-in.  A third party library that's undermaintained (and uses dynamic
> props) won't be able to be used out of the box once we apply this.  I =
don't
> think that's enough to scare us off, it just means that the opt-in sid=
e of
> that argument cancels out.
>=20
> -Sara

I am overall in favor of this move and making declared-only the default.=
  What I'm still undecided on is what escape hatch should be allowed.  I=
 agree that "you can extend stdClass already" isn't a good one, because =
that isn't viable in many cases.

The most viable options in my mind are "use __get/__set yourself, you he=
athen" and an attribute.  The benefit of an attribute over an interface =
or new parent class is that it's silently backward compatible, especiall=
y since attributes are comments in <8.0 versions, so legacy code can add=
 that marker and still be compatible with anything from 5.0 on.

The "internal impact" section makes me lean toward the "__get/__set" opt=
ion, but I don't fully understand the benefits of the impact.  It seems =
it would make the engine a little tidier and more efficient, but I don't=
 have a good sense for how much more efficient, which would need to be b=
alanced against how much more INefficient user-space code with dynamic p=
roperties would become.  (Either using __get/__set itself, or inheriting=
 from a future stdClass that implements them internally.)  IMO we should=
 prioritize engine efficiency over edge-case efficiency (which dynamic p=
roperties are these days), but the amount still matters.

I'm currently working on some improvements to the FIG to allow it to rel=
ease small, incomplete code libraries or shared type libraries.  Perhaps=
 a fully user-space BC trait with the basic behavior (basically what's s=
hown in the RFC) could live there?  (Not speaking on FIG's behalf, here,=
 just musing aloud.)

--Larry Garfield