Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116268
To: internals@lists.php.net
References: <CAF+90c9_qu=PiTS81J2mesPfJdz7n5nSi3qoLNLjhEGN-aTytA@mail.gmail.com>
 <CAF+90c-jw48ymXvO34E+oVua6_FV0NhwE_jnPgBZ=LurthnKeA@mail.gmail.com>
 <a2719507-7d39-ef12-f87f-0570de0094e2@wikimedia.org>
Message-ID: <ee9da872-afe2-002c-7376-0d65a99a898a@gmail.com>
Date: Wed, 13 Oct 2021 09:19:47 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <a2719507-7d39-ef12-f87f-0570de0094e2@wikimedia.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-GB
Subject: Re: [PHP-DEV] Re: [RFC] Deprecate dynamic properties
From: rowan.collins@gmail.com (Rowan Tommins)

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]