What to do with opaque GMP objects which allows instantiation

php.internals

Girgias

3 years ago
Hello internals, While working on something, I was using GMP for a test case and creating it by doing akin to $o = new GMP(5); However, this does *not* create a GMP object initialized to 5, rather it creates and sets the value to 0. This is because it does not define a constructor, and the proper way to create GMP objects is to use the procedural gmp_init() API. GMP was one of the first resource to object conversions which was performed in PHP 5.6. More recent resource to object conversions prevent the instantiation of the opaque object via new by defining a constructor that throws an Error to point to the procedural API. (something which would have saved me at least 30 minutes of debug time) Opened an issue [1] and a PR was made [2] which I requested to target 8.1 as I was considering this a bugfix, however it seems that some people to initialize the object by just writing new GMP(). Thus, the question, what should we do? - Merge as a bug fix for 8.1. - Target master (or 8.2.1) and align the behaviour with other opaque objects. - Add a proper constructor that does the same as gmp_init() in master/8.2.1 - Add a mock constructor that allows instantiating without arguments, but throws an error if trying to provide an argument. - Do nothing Best regards, George P. Banyard [1] https://github.com/php/php-src/issues/10155 [2] https://github.com/php/php-src/pull/10158

Tim Düsterhus

3 years ago
Hi On 12/30/22 18:36, G. P. B. wrote:
> - Add a proper constructor that does the same as gmp_init() in master/8.2.1
My preference would be this (without any target branch preference): If there is no technical reason to disallow creating new GMP objects via 'new', then I believe that making those objects behave more like "regular" / "userland" objects is a good thing. Best regards Tim Düsterhus

Hans Henrik Bergan

3 years ago
Why isn't that generating an ArgumentCountError? On Fri, Dec 30, 2022, 21:36 Tim Düsterhus <tim@bastelstu.be> wrote:

Larry Garfield

3 years ago
On Fri, Dec 30, 2022, at 2:36 PM, Tim Düsterhus wrote:
> Hi > > On 12/30/22 18:36, G. P. B. wrote: >> - Add a proper constructor that does the same as gmp_init() in master/8.2.1 > > My preference would be this (without any target branch preference): > > If there is no technical reason to disallow creating new GMP objects via > 'new', then I believe that making those objects behave more like > "regular" / "userland" objects is a good thing.
I concur. If I'm supposed to use it as an object, let me use it as an object. I defer to the relevant release managers as to what versions it should be fixed in, but I agree this is the right fix. --Larry Garfield