Re: [PHP-DEV] [RFC] [Discussion] Rounding Integers as int
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
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()
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()
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()
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()
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()
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
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()
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
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