Re: [PHP-DEV] What to do with opaque GMP objects which allows instantiation

2023-01-01 Thread Larry Garfield
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] What to do with opaque GMP objects which allows instantiation

2022-12-30 Thread Hans Henrik Bergan
Why isn't that generating an ArgumentCountError?

On Fri, Dec 30, 2022, 21:36 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.
>
> Best regards
> Tim Düsterhus
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>


Re: [PHP-DEV] What to do with opaque GMP objects which allows instantiation

2022-12-30 Thread Tim Düsterhus

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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



[PHP-DEV] What to do with opaque GMP objects which allows instantiation

2022-12-30 Thread G. P. B.
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