Re: [PHP-DEV] [RFC] [Discussion] Rounding Integers as int

2023-10-15 Thread Jakub Zelenka
Hi Marc,

On Sat, Oct 14, 2023 at 1:21 AM Marc Bennewitz  wrote:

> Hi Jakub,
>
> On 13.10.23 13:25, Jakub Zelenka wrote:
> > On Tue, Sep 26, 2023 at 11:39 AM Marc Bennewitz 
> wrote:
> >
> >> Hi internals
> >>
> >> I'd like to put a new RFC under discussion:
> >> https://wiki.php.net/rfc/integer-rounding
> >>
> >>
> > I would personally prefer a new function for rounding integers if anyone
> > wants to round large integers.
> >
> > The things is that the current round behaviour is consistent with a way
> how
> > floats to int conversion is done for strict types.
> >
> >  > declare(strict_types=1);
> >
> > function test(float $a) {
> >  echo $a;
> > }
> > test(987654321098765432);
> >
> >
> > So it won't really help that much if this function returns long for long
> in
> > cases where the result is passed to another function expecting float.
> >
> > The main problem that I see with the current approach is that it changes
> > return type for an edge case that most people are not impacted with. For
> > example it is quite usual that people had a float value with 0 fraction
> > which gets json encode to int. When they decode it and use round, the
> > return type is suddenly int. Even though it's usually not a problem,
> there
> > might be some code that expects float and maybe even assert that with
> > is_float. Such code will break.
> >
> > On the other hand I see use some case for rounding large integers with
> > negative precision. But for that to work I think it would be better to
> have
> > a special function.
>
> Your JSON example is a bit unrelated because if you care about your
> types your should have used JSON_RESERVE_ZERO_FRACTION in the first
> place else you should not care about int vs float at all.
>
>
The thing is that JSON might be encoded by another application that doesn't
care about types that much. In fact in JSON there is no difference between
2.0 and 2. So this is quite possible situation IMHO.


> It's true that passing/returning int to/from a function expecting float
> will cast but currently with these rounding functions it's a different
> deal as they expect an `int|float` instead of just `float`. So it's not
> cast on passing the argument but the functions itself are casting.
>
>
Well internally yeah but effectively it is a cast on the return type. What
I meant is that this wont help everywhere when it's passed to function
expecting just float unless it is changed by user to accept int|float.

Where another set of functions would avoid the BC break it also would be
> against having PHP as a loosely typed languageputting the burden to
> everyone caring. I already see hundreds of `is_int($v) ? round_int($v) :
> round($v)` everywhere around.
>
>
But for me it's sort of an edge as I would think it is not that usual
rounding values that are greater than 2^53. I think it would be good to
note in the RFC what other language do about this issue. I quickly checked
Go and it seems to only have float64 rounding (Math.Round). Rust seems to
also round just f64.

I'm starting to feel that the problem is that the input type is defined as
int|float. I think we should just correct it to float.

Regards

Jakub


[PHP-DEV] Re: DOMXPath / XSLTProcessor function callbacks

2023-10-15 Thread Frederik Bosch

Dear Niels,

First of all, thanks for all your hard work already on the DOM and 
SimpleXML extensions. I have been following your work in PHP-SRC, great! 
I am the author of this XSL 2.0 Transpiler in PHP package 
(https://github.com/genkgo/xsl). It is indeed possible to use 
workarounds for closures or object methods. I am relying on them in my 
package.


My suggestion for the future would be to add the following method.

public XSLTProcessor::registerFunctionsNS(string $namespace, 
array|ArrayAccess $functions): void


Then a user can register functions like this.

$xsltProcessorOrDomXpath->registerFunctionsNS('urn:my.namespace', 
array('upper-case', 'strtoupper', 'pow' => fn ($a, $b) => 
$a[0]->textContent ** $b[0]->textContent, 'other' => [$obj, 'method']);


The registered functions should use the same methodology as 
php:function(). Hence, string casting of arguments is something the 
library user should do. I would leave registerPHPFunctions as is, and 
maybe discourage it in favor of the method above. What if both are 
called? I think it would be most clear if the registerFunctionsNS method 
throws InvalidArgumentException when http://php.net/xsl or 
http://php.net/xpath is passed as namespace.


Cheers,
Frederik


On 13-10-2023 00:39, Niels Dossche wrote:

I'm looking to extend the functionality of calling PHP functions from within 
the DOMXPath or XSLTProcessor classes.

In case you're unfamiliar here's a quick rundown.
The DOMXPath class allows you to execute XPath queries on a DOM tree to lookup 
certain nodes satisfying a filter.
PHP allows the user to execute function callbacks within these. For example 
(from the manual):
   $xpath->query('//book[php:functionString("substr", title, 0, 3) = "PHP"]');
This will read the title element's text content, call substr on it, and then compare the 
output against "PHP".
You can not only call builtin functions, but also user functions.

To be able to call PHP functions, you need to use 
DOMXPath::registerPhpFunctions() 
(https://www.php.net/manual/en/domxpath.registerphpfunctions.php).
You either pass in NULL to allow all functions, or pass in which function names 
are allowed to be called.

Similarly, XSLTProcessor has the same registerPhpFunctions() method.
For XSLT it's mostly used for performing arbitrary manipulations on input data.
Normally the output of the function is put into the resulting document.


So what's the problem?
The current system doesn't allow you to call closures or object methods.
There are tricks you can do with global variables and global functions to try 
to work around this, but that's quite cumbersome.

There are two feature requests for this on the old bugtracker:
   - https://bugs.php.net/bug.php?id=38595
   - https://bugs.php.net/bug.php?id=49567

It's not hard to implement support for this, the question is just what API we 
should go with.
Based on what I've read, there are at least two obvious options:


OPTION 1) Extend registerPHPFunctions() such that you can pass in callables

```
// Adapted from https://bugs.php.net/bug.php?id=38595
$xslt->registerPHPFunctions(array(
'functionblah', // Like we used to
'func2' => fn ($x) => ...,
'func3' => array($obj, 'method'), // etc
));
```

Example: Using php:function("func3") inside XPath/XSLT in this case will result 
in calling method on $obj.
Similarly func2 will call the closure, and functionblah in the snippet just 
allowlists calling functionblah.

It's a backwards compatible solution and a natural extension to the current 
method.
It may be hard to discover this feature compared to having a new API though.

Furthermore, once you pass in function names to registerPHPFunctions(), you're 
restricting what can be called.
For example: imagine you want to call both ucfirst() and $obj->method(), so you 
pass in an entry like func3 in the above example.
Now you have to pass in ucfirst to registerPHPFunctions() too, because 
registerPHPFunctions() acts as an allowlist. May be a bit inconvenient.


OPTION 2) Add new methods to register / unregister callables

This may be the cleaner way to go about it on first sight, but there's a 
potential BC break when new methods clash in user-defined subclasses.

Question here is: what about the interaction with registerPHPFunction?
What if both registerPHPFunction() and the register method add something with 
the same name?
What if registerPHPFunction() didn't allowlist a function but the register 
method added it, may be a bit confusing for users.
The interaction may be surprising.



Please let me know your thoughts.

Cheers
Niels



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



Re: [PHP-DEV] Better name for method Randomizer::nextFloat()

2023-10-15 Thread Tim Düsterhus

Hi

On 10/15/23 02:04, tag Knife wrote:

I think the major pet peeve i have is that the "Randomizer" class is
encapsulating all random functions.
I would like to see each randomizer have its own class, since they are
under the Random namespace
anyway, I believe they should be Random/StringRandomizer,
Random/IntRandomizer,
Random/FloatRandomizer, so on so forth. Basically Ramdom/{Type}Randomizer.
This would
also allow more granular and specific functionality for each type.

For example, for this discussion, the class, Random/FloatRandomizer (and
IntRandomizer)
could be instatiated with its boundries.

```php
$randomFloat = new Random/FloatRandomizer($lowerBound, $upperBound,
$boundryInterval)
$a = $randomFloat->get();
$b = $randomFLoat->next();
```

Is that not more intuitive?



Creating a separate class for each possible distribution would certainly 
be the cleanest API from an "academic" point of view. I'm not sure if it 
would be more intuitive, but I'm sure that it would not be easier to 
use. The API of Random\Randomizer is probably not perfect, but I believe 
it does quite a few things quite right:


- It is secure by default, if you do not provide an engine, it defaults 
to the CSPRNG.
- It is discoverable, your IDE will autocomplete the available methods 
and all the methods are listed right beneath each other in the 
documentation.

- It is succinct for common cases and (mostly) does what it says on the tin.
- It allows you to easily build additional (userland) APIs on top of it, 
thus serving as a "building block" (to reuse the phrasing from the 
Randomizer additions RFC). Such a userland API could look like the API 
you proposed (though it would probably make sense to also include the 
"Uniform" somewhere within the name, because you could also have a 
"Normal" distribution, so we're right in bike-shedding territory, API 
design is hard).


To give an example:

// Inspired by Rust's API.
$engine = new \Random\Engine\Secure();
$oneToHundred = new \Random\Distribution\UniformInt(1, 100);
$randomInt = $oneToHundred($engine);
$anotherRandomInt = $oneToHundred($engine);

vs

// The default PHP API.
$randomizer = new \Random\Randomizer();
$randomInt = $randomizer->getInt(1, 100);
$anotherRandomInt = $randomizer->getInt(1, 100);

I would certainly prefer PHP's API for most of my applications, because 
it's much less boilerplate. It also avoids constructing many short-lived 
objects [1]: You can just throw both the Engine and Randomizer into your 
dependency injection container.


Best regards
Tim Düsterhus

[1] I expect the performance impact of constructing and destructing 
objects to be greater than a regular function call, but might be wrong here.


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



Re: [PHP-DEV] Better name for method Randomizer::nextFloat()

2023-10-15 Thread Tim Düsterhus

Hi

On 10/14/23 19:47, David Grudl wrote:

random number in the interval 0...1, so it does the same thing as
getFloat(0, 1).  See
https://wiki.php.net/rfc/randomizer_additions#nextfloat


For the record: That is correct.


B) When trying to think of a more appropriate name (e.g.
`getFloat01()` ?), I find that the simple `getFloat(0, 1) is concise
and succinct enough. If it were really useful to have a method that
returns numbers in this interval, wouldn't it be better to give the
$min and $max parameters the default values of 0 and 1, and thus call
`getFloat()` directly?


Yes, the right-open interval [0, 1) is useful to have as a special case, 
because it directly maps onto percentages, allowing you to easily 
generate booleans with a specific chance.


Making those default values of getFloat() wouldn't be better, though. It 
is not meaningful to only provide *one* of the boundaries (specifically 
$min), but PHP does not support overloaded signatures without 
func_num_args() trickery and this was banned for internal functions as 
of this RFC:


https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures#policy_about_new_functions

Making getFloat(float $min = 0.0, float $max = 1.0, IntervalBoundary 
$boundary = IntervalBoundary::ClosedOpen) would seemingly make it legal 
to call ->getFloat(0.5), which I consider to be worse than nextFloat().


Side note: The implementation of nextFloat() is much more efficient than 
that of getFloat(0.0, 1.0) and the output will differ even for the same 
seeded engine. The domain of legal return values is identical.



C) PHP 8.3 is still in testing and this change can be made. If it is
not done now, it will never be done and programmers will have to use
and also read unintuitive APIs for years. Similarly, in the days
before PHP 8.0 was released, I suggested changing PhpToken::getAll()
to today's tokenize() and there was no problem with that
https://bugs.php.net/bug.php?id=80328


The release managers would need to decide whether a change would be 
legal at this point in the release process, but I strongly doubt it, 
especially since naming is inherently bike-shedding territory and since 
the method name is already in various blog articles and in the 
documentation.


For the same reason I'm not sure if comparing this to PhpToken::getAll() 
/ tokenize() is a useful comparison, since the tokenizer is a very 
specialized feature that only few developers will care about. Making a 
breaking change late in the release process will have a much smaller 
impact to existing documentation / blog articles / feature videos / etc.


Best regards
Tim Düsterhus

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



Re: [PHP-DEV] Two new functions array_first() and array_last()

2023-10-15 Thread Saki Takamachi
Hi Pierre

> The first two probably only make sense for a numerically indexed array

I do not think so. If these are something like "reset/end without side 
effects", then they should work fine even not numeric indexed array.

Regards.

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



Re: [PHP-DEV] Two new functions array_first() and array_last()

2023-10-15 Thread Larry Garfield
On Sun, Oct 15, 2023, at 7:40 AM, Pierre wrote:


> The first two probably only make sense for a numerically indexed array, 
> so I guess that array_is_list() (whatever the name is, I don't want to 
> bikeshed about naming) would be a good addition as well, that, in my 
> opinion, would be pertinent to add at the same time.

That has already been done: https://www.php.net/array_is_list

--Larry Garfield

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



Re: [PHP-DEV] Two new functions array_first() and array_last()

2023-10-15 Thread Pierre

Le 15/10/2023 à 01:11, Ben Ramsey a écrit :

On Oct 14, 2023, at 16:30, Nikita Popov  wrote:

On Sat, Oct 14, 2023, at 20:00, David Grudl wrote:

PHP lacks two very basic functions for working with arrays:

- array_first() returning the first element of an array (or null)
- array_last() returning the last element of the array (or null)

While PHP has functions that return the first and last keys,
array_key_first() and array_key_last(), it does not have more useful
functions for values.

a) What about reset() and end()?
Programmers "abuse" the reset() and end() functions for this purpose.
The problem is that these functions are used to move the internal
pointer in the array. Which is why they have a name that is
inappropriate when used in the sense of "return me the first element".

Much worse, they shouldn't to be used to get first/last value, because
they have a side effect (i.e. moving the pointer).

Further, in the absence of an element, they return the obsolete false
and not the currently expected null, which can be combined with the ??
operator. In this they differ from the similar functions
array_key_first() and array_key_last().

b) What about $array[array_key_first($array)]?

For such basic functions as returning the first and last item in an
array, there should be a function in the basic package, not a
workaround. Moreover, this requires having the array in a local
variable, since $this->getFoo()[array_key_first($this->getFoo())]
would be very inefficient and possibly incorrect.

c) Two such functions were proposed and rejected during the
array_key_first/last RFC
(https://wiki.php.net/rfc/array_key_first_last)

Yes, that was in 2018. At that time, functions like str_contains() or
str_starts_with() wouldn't have even come into existence, just because
there was an obscure way to do it without them. I believe we've moved
on since then. Today we know how useful it is to use simple,
easy-to-understand methods, both for programmers who write and read
the code.

DG

I'm in favor of adding these.

To add to what you already said, because reset/end modify the array, there's a 
good chance that calling these functions will copy the whole array due to a 
modification you are not actually interested in.

So basically you have the choice between calling end(), which is the wrong 
thing to do semantically and may be slow, or using 
$array[array_key_last($array)], which is rather convoluted, and incorrect if 
the array is potentially empty.

Regards,
Nikita

I’m in favor of these functions, for all the same aforementioned reasons.


Yes please !

array_first() and array_last() are definitely needed. I wrote `foreach 
($foo as $value) break;` too many times in my life. array_key_first() 
and array_key_last() I wouldn't use it much, but they'd probably  find 
their use cases as well.


The first two probably only make sense for a numerically indexed array, 
so I guess that array_is_list() (whatever the name is, I don't want to 
bikeshed about naming) would be a good addition as well, that, in my 
opinion, would be pertinent to add at the same time.


Regards,

Pierre

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



Re: [PHP-DEV] DOMXPath / XSLTProcessor function callbacks

2023-10-15 Thread Niels Dossche
Hi Marc

On 10/15/23 12:53, Marc wrote:
> Hi,
> 
> On 14.10.23 13:54, Niels Dossche wrote:
>> Hi Tim
>>
>> On 10/14/23 12:30, Tim Düsterhus wrote:
>>> Hi
>>>
>>> On 10/13/23 00:39, Niels Dossche wrote:
 Please let me know your thoughts.
>>> What does calling ->registerPHPFunctions() do when it's called more than 
>>> once? Will the existing allow-list be overwritten or amended?
>>>
>>> i.e.
>>>
>>> $xpath->registerPHPFunctions([
>>>    'strtoupper',
>>> ]);
>>> $xpath->registerPHPFunctions([
>>>    'ucfirst',
>>> ]);
>>>
>>> will I be able to:
>>>
>>> (a) Call ucfirst(), but not strtoupper()
>>> (b) Call both
>> You can call both, it's additive.
> 
> As it's not clear what these functions do wouldn't it make more sense to 
> introduce new / more clear functions and deprecate the old?
> 
> Like:
> 
> ->addXPathCallable(string $alias, callable $fn);
> ->getXPathCallables(): array
> ->removeXPathCallable(string $alias);

This would be the cleaner option.
The question here is how they would interact with each other and the existing 
registerPhpFunctions():
- addXPathCallable() when the alias already exists: I guess throw a ValueError
- remove and get should likely also act on the functions registered via 
registerPhpFunctions()
- What should happen when we call registerPhpFunctions() followed by 
addXPathCallable("foo", function ($...) {...});
  Probably both global functions and foo should be callable. But what should 
happen then when there is also a global function foo, which one should be 
called? Probably the explicitly registered callable?

What's currently also weird is that in the following snippet:
```
$xpath->registerPhpFunctions();
$xpath->registerPhpFunctions("ucfirst");
```
then only ucfirst() would be callable, which is surprising especially 
considering that calling $xpath->registerPhpFunctions("otherfunction") 
afterwards will make both ucfirst and otherfunction callable.

> 
> And is it really necessary to be able to register all functions? Also what 
> would happen in this case with functions included later? Would they also be 
> callable or only the functions known at the point of registering?
> 

For the first question: unlikely.
For the second question: they would also be callable.

>>> Best regards
>>> Tim Düsterhus
>> Cheers
>> Niels
> 
> 
> Best,
> Marc
> 

Cheers
Niels

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



Re: [PHP-DEV] Two new functions array_first() and array_last()

2023-10-15 Thread Paul Dragoonis
On Sun, 15 Oct 2023, 04:49 Saki Takamachi,  wrote:

> I came up with the idea of ​​using a signature like array_filter(), and
> when a callback is passed, "return the first/last element that matches the
> condition" and "return null if there is no match."
>
> The downside to this idea is the loss of simplicity. So I'll leave it up
> to you whether you want to go with this idea or not. I have no intention of
> forcing this.
>
> Best regards.
>

I'm in favor of these functions.

Right now I am doing:

$firstItem = current($array)
$secondItem = next($array)

Which is simple and works, but really it's just by design that the code
hasn't shifted the array pointer yet and I can cheat with current()

If the pointer was shifted then I'd have to do reset(), which isn't good.

array_first() for the win

Many thanks,
Paul




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


Re: [PHP-DEV] DOMXPath / XSLTProcessor function callbacks

2023-10-15 Thread Marc

Hi,

On 14.10.23 13:54, Niels Dossche wrote:

Hi Tim

On 10/14/23 12:30, Tim Düsterhus wrote:

Hi

On 10/13/23 00:39, Niels Dossche wrote:

Please let me know your thoughts.

What does calling ->registerPHPFunctions() do when it's called more than once? 
Will the existing allow-list be overwritten or amended?

i.e.

$xpath->registerPHPFunctions([
   'strtoupper',
]);
$xpath->registerPHPFunctions([
   'ucfirst',
]);

will I be able to:

(a) Call ucfirst(), but not strtoupper()
(b) Call both

You can call both, it's additive.


As it's not clear what these functions do wouldn't it make more sense to 
introduce new / more clear functions and deprecate the old?


Like:

->addXPathCallable(string $alias, callable $fn);
->getXPathCallables(): array
->removeXPathCallable(string $alias);

And is it really necessary to be able to register all functions? Also 
what would happen in this case with functions included later? Would they 
also be callable or only the functions known at the point of registering?



Best regards
Tim Düsterhus

Cheers
Niels



Best,
Marc