Re: [PHP-DEV] [RFC] [Discussion] Clone with

2023-05-30 Thread Alexandru Pătrănescu
On Wed, May 31, 2023 at 2:53 AM Larry Garfield 
wrote:

> On Tue, May 30, 2023, at 10:04 PM, Alexandru Pătrănescu wrote:
> > On Tue, May 30, 2023, 19:39 Larry Garfield 
> wrote:
> >
> >> On Mon, May 29, 2023, at 11:22 PM, Máté Kocsis wrote:
> >> > To be honest, the current behavior seemed like the natural choice for
> >> > me, and I didn't really consider to execute the __clone() method after
> >> the
> >> > clone assignments.
> >> > Do you have a use-case in mind when you need to forward-pass
> information
> >> to
> >> > __clone()?
> >>
> >> Not a specific one off hand.  It's more a conceptual question.  `with`
> has
> >> more contextual awareness than __clone(), so it should have "first
> crack"
> >> at the operation, so that if necessary it can make changes that
> __clone()
> >> can then respond to.  The inverse doesn't make sense.
> >>
> >> The only reason for `with` to come after would be to allow `with` to
> >> "override" or "undo" something that __clone() did.  Generally speaking,
> if
> >> you have to undo something you just did, you shouldn't have done it in
> the
> >> first place, so that's a less compelling combination.
> >>
> >> This one isn't a deal breaker, but we should be sure to think it through
> >> as it's kinda hard to reverse later.
> >>
> >
> > To me so far also it was natural to assume that __clone is first and only
> > after that the rest of the operations.
> > But `with` operations, be it properties assignment or even a closure,
> would
> > run in the context of the caller of clone and sometimes this might be run
> > not from a method of the cloned class.
> >
> > An example:
> > There is a class that represents persons of a fictive country/planet.
> > Each person has many properties but has also a first name and a last name
> > and there is a rule: the two names must not start with the same letter.
> > Both names cannot be changed as they are defined readonly.
> > Creation of new persons can be done using new for new random properties
> or
> > using clone to preserve existing properties. But in both cases the first
> > name and last name are randomly chosen.
> > If we want to control the last name value during clone that would be
> > possible using the `with` operation but the logic to allocate a first
> name
> > will only happen in `__clone()`method.
> >
> > To be able to achieve this we must have __clone last, as there we have
> the
> > internal validations, operations and also access to private/protected
> > members that are not accesible from where clone is being called.
> >
> > Regards,
> > Alex
>
> I... could not understand that in the slightest.  Can you show it in code?
>
>

Sorry for that. Here you go: https://3v4l.org/JIBoI/rfc#vgit.master
If __clone would be first, there is no way to enforce the rule that a
person cannot have their first name starting with the same letter as last
name.

Regards,
Alex


Re: [PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Boro Sitnikovski


> On 30.5.2023, at 21:56, Andreas Hennings  wrote:
> 
> On Tue, 30 May 2023 at 19:25, Boro Sitnikovski  wrote:
>> 
>> Here's some more examples:
>> 
>> 1. Use `array_group` to create list of singleton list:
>> ```
>> $groups = array_group( $arr, function( $p1, $p2 ) {
>>  return false;
>> } );
>> ```
>> 
>> (This can also be achieved with `array_map` returning `[ $x ]`)
>> 
>> 2. Distinct groups for consecutive positive and negative elements
>> 
>> ```
>> $arr = [-1,2,-3,-4,2,1,2,-3,1,1,2];
>> 
>> $groups = array_group( $arr, function( $p1, $p2 ) {
>>  return ($p1 > 0) == ($p2 > 0);
>> } );
>> ```
>> 
>> This produces `[[-1],[2],[-3,-4],[2,1,2],[-3],[1,1,2]]`, so we can easily 
>> capture the groups of highs/lows for example.
>> 
>> 3. Group sentences (similar to `explode`, but still different)
>> 
>> ```
>> $arr = "Hello, PHP. Good to see you.";
>> $groups = array_group( str_split( $arr ), function( $p1, $p2 ) {
>>return '.' !== $p1;
>> } );
>> 
>> $groups = array_map( 'join', $groups );
>> ```
>> 
>> Producing `[ "Hello, PHP.", " Good to see you." ]`.
>> 
>> 4. Grouping book sections
>> ```
>> $book_sections = [ '1.0', '1.1', '1.2', '2.0', '2.1', '3.0', '3.1' ];
>> $groups = array_group( $book_sections, function( $p1, $p2 ) {
>>return $p1[0] === $p2[0];
>> } );
>> ```
>> 
>> Producing `[ [ '1.0', '1.1', '1.2' ], [ '2.0', '2.1'], [ '3.0', '3.1' ] ]`
>> 
>> and so on...
>> 
>> Basically, it's a very general utility :)
>> 
>> Best,
>> 
>> Boro
> 
> Only one of these examples actually uses a comparison function that
> actually compares two adjacent values.
> All the others look at a single value only, or they don't care about
> the original order at all.
> 
> And that example (2) seems quite artificial and rare.
> 
>> so we can easily capture the groups of highs/lows for example
> 
> How often does this problem come up in practice?
> 
> I did actually have a look at the Haskell doc page you linked.
> https://hackage.haskell.org/package/groupBy-0.1.0.0/docs/Data-List-GroupBy.html
> I wonder where this is used in the Haskell world, and how often they
> need this true comparison of adjacent values.
> The examples from the doc page are quite artificial. That's ok for a
> doc page, but does not make a good argument for why we need it.

Thanks for digging deeper in trying to understand the value that this would 
bring.

For a more realistic example, check the following gist: 
https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_datetime_group-php.
 This was a very recent thing I worked on in production. The events came 
ordered from the database. But also, this is not the first time I was in need 
of a grouping functionality.

Sorry for repeating, but I see it as a handy utility. We can bring the same 
argument for any of the `array_*` functions because most of them can be 
implemented with for loops.

If the main concern is the consecutive processing of the elements, we could add 
a third argument to `array_group` (flag) that would allow users to alter the 
processing behaviour - at a price, consecutive taking O(n) vs all elements 
taking something close to O(n^2)
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] [Discussion] Clone with

2023-05-30 Thread Larry Garfield
On Tue, May 30, 2023, at 10:04 PM, Alexandru Pătrănescu wrote:
> On Tue, May 30, 2023, 19:39 Larry Garfield  wrote:
>
>> On Mon, May 29, 2023, at 11:22 PM, Máté Kocsis wrote:
>> > To be honest, the current behavior seemed like the natural choice for
>> > me, and I didn't really consider to execute the __clone() method after
>> the
>> > clone assignments.
>> > Do you have a use-case in mind when you need to forward-pass information
>> to
>> > __clone()?
>>
>> Not a specific one off hand.  It's more a conceptual question.  `with` has
>> more contextual awareness than __clone(), so it should have "first crack"
>> at the operation, so that if necessary it can make changes that __clone()
>> can then respond to.  The inverse doesn't make sense.
>>
>> The only reason for `with` to come after would be to allow `with` to
>> "override" or "undo" something that __clone() did.  Generally speaking, if
>> you have to undo something you just did, you shouldn't have done it in the
>> first place, so that's a less compelling combination.
>>
>> This one isn't a deal breaker, but we should be sure to think it through
>> as it's kinda hard to reverse later.
>>
>
> To me so far also it was natural to assume that __clone is first and only
> after that the rest of the operations.
> But `with` operations, be it properties assignment or even a closure, would
> run in the context of the caller of clone and sometimes this might be run
> not from a method of the cloned class.
>
> An example:
> There is a class that represents persons of a fictive country/planet.
> Each person has many properties but has also a first name and a last name
> and there is a rule: the two names must not start with the same letter.
> Both names cannot be changed as they are defined readonly.
> Creation of new persons can be done using new for new random properties or
> using clone to preserve existing properties. But in both cases the first
> name and last name are randomly chosen.
> If we want to control the last name value during clone that would be
> possible using the `with` operation but the logic to allocate a first name
> will only happen in `__clone()`method.
>
> To be able to achieve this we must have __clone last, as there we have the
> internal validations, operations and also access to private/protected
> members that are not accesible from where clone is being called.
>
> Regards,
> Alex

I... could not understand that in the slightest.  Can you show it in code?

--Larry Garfield

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



Re: [PHP-DEV] [RFC] [Discussion] Clone with

2023-05-30 Thread Alexandru Pătrănescu
On Tue, May 30, 2023, 19:39 Larry Garfield  wrote:

> On Mon, May 29, 2023, at 11:22 PM, Máté Kocsis wrote:
> > To be honest, the current behavior seemed like the natural choice for
> > me, and I didn't really consider to execute the __clone() method after
> the
> > clone assignments.
> > Do you have a use-case in mind when you need to forward-pass information
> to
> > __clone()?
>
> Not a specific one off hand.  It's more a conceptual question.  `with` has
> more contextual awareness than __clone(), so it should have "first crack"
> at the operation, so that if necessary it can make changes that __clone()
> can then respond to.  The inverse doesn't make sense.
>
> The only reason for `with` to come after would be to allow `with` to
> "override" or "undo" something that __clone() did.  Generally speaking, if
> you have to undo something you just did, you shouldn't have done it in the
> first place, so that's a less compelling combination.
>
> This one isn't a deal breaker, but we should be sure to think it through
> as it's kinda hard to reverse later.
>

To me so far also it was natural to assume that __clone is first and only
after that the rest of the operations.
But `with` operations, be it properties assignment or even a closure, would
run in the context of the caller of clone and sometimes this might be run
not from a method of the cloned class.

An example:
There is a class that represents persons of a fictive country/planet.
Each person has many properties but has also a first name and a last name
and there is a rule: the two names must not start with the same letter.
Both names cannot be changed as they are defined readonly.
Creation of new persons can be done using new for new random properties or
using clone to preserve existing properties. But in both cases the first
name and last name are randomly chosen.
If we want to control the last name value during clone that would be
possible using the `with` operation but the logic to allocate a first name
will only happen in `__clone()`method.

To be able to achieve this we must have __clone last, as there we have the
internal validations, operations and also access to private/protected
members that are not accesible from where clone is being called.

Regards,
Alex


Re: [PHP-DEV] Declaration-aware attributes

2023-05-30 Thread Andreas Hennings
On Tue, 30 May 2023 at 22:45, Larry Garfield  wrote:
>
> On Tue, May 30, 2023, at 7:34 PM, Andreas Hennings wrote:
> > On Tue, 30 May 2023 at 19:12, Larry Garfield  wrote:
> >>
> >> I've run into this issue in my attribute library as well 
> >> (https://github.com/Crell/AttributeUtils).  What I do there is allow 
> >> attributes to opt-in to a callback method via interface.  For example:
> >>
> >> #[\Attribute]
> >> class AttribWithName implements FromReflectionClass
> >> {
> >> public readonly string $name;
> >>
> >> public function __construct(?string $name = null)
> >> {
> >> if ($name) {
> >> $this->name = $name;
> >> }
> >> }
> >>
> >> public function fromReflection(\ReflectionClass $subject): void
> >> {
> >> $this->name ??= $subject->getShortName();
> >> }
> >> }
> >
> > So it is technically from outside it is a setter, whereas from inside
> > it is not really.
> > Technically, this means you need a version of the attribute object
> > that can exist without the values from the reflector.
> > The constructor needs to initialize some "stub" values, until the
> > setter is called.
> > Every other method also needs to support the case when the setter has
> > not been called yet, or possibly throw an exception.
> > Also, your property type has to allow null, so `?string`, not `string`.
>
> Except the property type is not null above?  The argument is, but not the 
> property.  (I could use CPP here with a null value instead, if we had 
> asymmetric visibility.)

True!
My conception of readonly was distorted by the same static analysis
tools that you complained about earlier!
https://3v4l.org/CUgYv
I think a more accurate label would be "write once".

>
> Also, if the interface is called by the reflection system itself as part of 
> getInstance() then yes, we can guarantee that it's been run, just like we can 
> guarantee the constructor has run.

The concern was that you can also construct the attribute object with
a regular `new AttribWithName()` expression, and then you can omit the
->fromReflection() call.
But we could say it is part of the contract that you have to call the
method before the object is fully operational.

>
> >> (Side note: This is why static analysis tools that forbid writing to 
> >> readonly properties except from the constructor are wrong; it's also an 
> >> example of where asymmetric visibility would be superior to readonly.  But 
> >> I digress.)
> >
> > Technically there is no guarantee that the setters will be called
> > before any other method, and only once.
> > If these methods can write on readonly properties, then any method can.
> > Unless we somehow mark these methods as special.
>
> There's nothing special about readonly properties there.  An uninitialized 
> non-readonly property is no more or less susceptible to still being 
> uninitialized when you need it.  Singling out readonly here is just dumb, and 
> exacerbates the problems of readonly.
>
> > On the other hand, a wither method with "clone with" should be allowed
> > to work on readonly properties.
> > You could rewrite your method like this, once we have clone with:
> > (or use the old-school syntax but without readonly)
> >
> > public function withReflector(\ReflectionClass $subject): static
> > {
> > return ($this->name !== NULL)
> > ? $this
> > : clone $this with (name: $subject->getShortName();
> > }
> >
> > Then in the discovery code:
> >
> > $attribute = $reflectionClass->getAttributes()[0]->newInstance();
> > $attribute = $attribute->withReflector($reflectionClass);
>
> In that library I actually have several opt-in post-constructor setup 
> routines.  The documentation covers them all.  Making them all withers would 
> be just needlessly slow.  Basically it's an example of a "mutable and then 
> locked" object, which I emulate by virtue of only calling the setup methods 
> from the library, and using readonly properties.
>
> >> That's why I think an opt-in interface is the way to go.  It also avoids 
> >> any confusion around the constructor parameters, which is, based on this 
> >> thread, a confusing area. :-)
> >
> > What do you think about a placeholder syntax to avoid confusion with a
> > skipped parameter?
> > Like
> >
> > #[A('x', ?', 'y')]
>
> Oh god no.  For one, function calls are already complicated enough and the 
> next thing we should add there is some kind of partial application, which 
> already discussed using ?.  Second, that the attribute cares about what it is 
> attached to is not something the caller should care about.  If the user of 
> the attribute needs to specify "and put the reflection object here" in the 
> attribute usage, we've failed.  I'd vote against any such proposal in any 
> form, whatever the syntax.

I was actually thinking of the partial application with the `?` symbol.
The metaphor would be that we create a partial callable from the `new
A(.., ?)` 

Re: [PHP-DEV] Declaration-aware attributes

2023-05-30 Thread Larry Garfield
On Tue, May 30, 2023, at 7:34 PM, Andreas Hennings wrote:
> On Tue, 30 May 2023 at 19:12, Larry Garfield  wrote:
>>
>> I've run into this issue in my attribute library as well 
>> (https://github.com/Crell/AttributeUtils).  What I do there is allow 
>> attributes to opt-in to a callback method via interface.  For example:
>>
>> #[\Attribute]
>> class AttribWithName implements FromReflectionClass
>> {
>> public readonly string $name;
>>
>> public function __construct(?string $name = null)
>> {
>> if ($name) {
>> $this->name = $name;
>> }
>> }
>>
>> public function fromReflection(\ReflectionClass $subject): void
>> {
>> $this->name ??= $subject->getShortName();
>> }
>> }
>
> So it is technically from outside it is a setter, whereas from inside
> it is not really.
> Technically, this means you need a version of the attribute object
> that can exist without the values from the reflector.
> The constructor needs to initialize some "stub" values, until the
> setter is called.
> Every other method also needs to support the case when the setter has
> not been called yet, or possibly throw an exception.
> Also, your property type has to allow null, so `?string`, not `string`.

Except the property type is not null above?  The argument is, but not the 
property.  (I could use CPP here with a null value instead, if we had 
asymmetric visibility.)

Also, if the interface is called by the reflection system itself as part of 
getInstance() then yes, we can guarantee that it's been run, just like we can 
guarantee the constructor has run.  

>> (Side note: This is why static analysis tools that forbid writing to 
>> readonly properties except from the constructor are wrong; it's also an 
>> example of where asymmetric visibility would be superior to readonly.  But I 
>> digress.)
>
> Technically there is no guarantee that the setters will be called
> before any other method, and only once.
> If these methods can write on readonly properties, then any method can.
> Unless we somehow mark these methods as special.

There's nothing special about readonly properties there.  An uninitialized 
non-readonly property is no more or less susceptible to still being 
uninitialized when you need it.  Singling out readonly here is just dumb, and 
exacerbates the problems of readonly.

> On the other hand, a wither method with "clone with" should be allowed
> to work on readonly properties.
> You could rewrite your method like this, once we have clone with:
> (or use the old-school syntax but without readonly)
>
> public function withReflector(\ReflectionClass $subject): static
> {
> return ($this->name !== NULL)
> ? $this
> : clone $this with (name: $subject->getShortName();
> }
>
> Then in the discovery code:
>
> $attribute = $reflectionClass->getAttributes()[0]->newInstance();
> $attribute = $attribute->withReflector($reflectionClass);

In that library I actually have several opt-in post-constructor setup routines. 
 The documentation covers them all.  Making them all withers would be just 
needlessly slow.  Basically it's an example of a "mutable and then locked" 
object, which I emulate by virtue of only calling the setup methods from the 
library, and using readonly properties.

>> That's why I think an opt-in interface is the way to go.  It also avoids any 
>> confusion around the constructor parameters, which is, based on this thread, 
>> a confusing area. :-)
>
> What do you think about a placeholder syntax to avoid confusion with a
> skipped parameter?
> Like
>
> #[A('x', ?', 'y')]

Oh god no.  For one, function calls are already complicated enough and the next 
thing we should add there is some kind of partial application, which already 
discussed using ?.  Second, that the attribute cares about what it is attached 
to is not something the caller should care about.  If the user of the attribute 
needs to specify "and put the reflection object here" in the attribute usage, 
we've failed.  I'd vote against any such proposal in any form, whatever the 
syntax.

>> My second preference would be the ReflectionAttribute::inProgress() call in 
>> the constructor, or something like that.  I like that less because it's a 
>> stateful call, but it would also reduce the issue with readonly properties 
>> (as in the example above) by making both alternatives available in the 
>> constructor, so maybe it's an acceptable tradeoff.
>
> I would like to avoid anything that is stateful or that leaves an
> incomplete stub object.
> (But I think I made this clear enough, so..)

I don't think it's going to be possible to have both.  We either have a static 
call of some kind (the inProgress() method above or similar) that only works 
sometimes (ie, while an attribute is being constructed), or we have multiple 
setup methods (which the engine can ensure are called, but technically that 
does mean the object has a moment where it's 

Re: [PHP-DEV] Declaration-aware attributes

2023-05-30 Thread Andreas Hennings
On Tue, 30 May 2023 at 19:12, Larry Garfield  wrote:
>
> I've run into this issue in my attribute library as well 
> (https://github.com/Crell/AttributeUtils).  What I do there is allow 
> attributes to opt-in to a callback method via interface.  For example:
>
> #[\Attribute]
> class AttribWithName implements FromReflectionClass
> {
> public readonly string $name;
>
> public function __construct(?string $name = null)
> {
> if ($name) {
> $this->name = $name;
> }
> }
>
> public function fromReflection(\ReflectionClass $subject): void
> {
> $this->name ??= $subject->getShortName();
> }
> }

So it is technically from outside it is a setter, whereas from inside
it is not really.
Technically, this means you need a version of the attribute object
that can exist without the values from the reflector.
The constructor needs to initialize some "stub" values, until the
setter is called.
Every other method also needs to support the case when the setter has
not been called yet, or possibly throw an exception.
Also, your property type has to allow null, so `?string`, not `string`.

I usually try to avoid this, this is why I proposed the constructor parameter.
This way, the object is never in an incomplete state.

(Side note: Personally I use the naming convention from*() for static
factory methods. I might use set*() for this one, but then again, it
is only a setter from the outside.)

>
> (Side note: This is why static analysis tools that forbid writing to readonly 
> properties except from the constructor are wrong; it's also an example of 
> where asymmetric visibility would be superior to readonly.  But I digress.)

Technically there is no guarantee that the setters will be called
before any other method, and only once.
If these methods can write on readonly properties, then any method can.
Unless we somehow mark these methods as special.

On the other hand, a wither method with "clone with" should be allowed
to work on readonly properties.
You could rewrite your method like this, once we have clone with:
(or use the old-school syntax but without readonly)

public function withReflector(\ReflectionClass $subject): static
{
return ($this->name !== NULL)
? $this
: clone $this with (name: $subject->getShortName();
}

Then in the discovery code:

$attribute = $reflectionClass->getAttributes()[0]->newInstance();
$attribute = $attribute->withReflector($reflectionClass);

>
> My preference would be for something along those lines to be implemented in 
> core.
>
> Importantly, we *MUST NOT* design it such that the reflection object gets 
> assigned to a property of the attribute.  Reflection objects are not 
> serializable.  Attributes will frequently be cached.  That means it forces 
> the attribute author to make the property nullable AND then unset it sometime 
> before the attribute gets serialized, or it will break.  That's a no-go.

There could be ways around this problem, but I agree we should avoid
it on design level.

>
> That's why I think an opt-in interface is the way to go.  It also avoids any 
> confusion around the constructor parameters, which is, based on this thread, 
> a confusing area. :-)

What do you think about a placeholder syntax to avoid confusion with a
skipped parameter?
Like

#[A('x', ?', 'y')]

>
> My second preference would be the ReflectionAttribute::inProgress() call in 
> the constructor, or something like that.  I like that less because it's a 
> stateful call, but it would also reduce the issue with readonly properties 
> (as in the example above) by making both alternatives available in the 
> constructor, so maybe it's an acceptable tradeoff.

I would like to avoid anything that is stateful or that leaves an
incomplete stub object.
(But I think I made this clear enough, so..)

>
> I can see an argument either direction here.
>
> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>

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



Re: [PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Boro Sitnikovski


> On 30.5.2023, at 19:21, Larry Garfield  wrote:
> 
> On Tue, May 30, 2023, at 4:27 PM, Boro Sitnikovski wrote:
>> Hi,
>> 
>> Thank you for your thoughts.
>> 
>>> I would say the more common desired behavior is the one in your first
>>> example. And even for that we don't have a native function.
>> 
>> This Google search might give more insight into the number of 
>> discussions about a grouping functionality: 
>> https://www.google.com/search?q=php+group+elements+site:stackoverflow.com
>> 
>>> Your behavior can be implemented in userland like so:
>>> https://3v4l.org/epvHm
>> 
>> Correct, but then again, we can also implement 
>> `array_map`/`array_filter`/etc. in userland :)
>> 
>>> I think you need to make a case as to why the behavior you describe
>>> justifies a native function.
>> 
>> Similar to my previous answer, but also in general - ease of access and 
>> also performance.
> 
> Do you have benchmarks showing that implementing it in C would be notably 
> faster?  That would help the case that it should be written in C.
> 
> Also, please do not top-post.
> 
> --Larry Garfield
> 
> -- 
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php

Sorry for the top-posting, my bad.

I ran some performance benchmarks and the improvement is about 25% - some 
details on the following gist: 
https://gist.github.com/bor0/3fa539263335fa415faa67606a469f2e?permalink_comment_id=4584327#gistcomment-4584327

Re: [PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Boro Sitnikovski
Hi,

> On 30.5.2023, at 18:33, Andreas Hennings  wrote:
> 
> On Tue, 30 May 2023 at 18:27, Boro Sitnikovski  wrote:
>> 
>> Hi,
>> 
>> Thank you for your thoughts.
>> 
>>> I would say the more common desired behavior is the one in your first
>>> example. And even for that we don't have a native function.
>> 
>> This Google search might give more insight into the number of discussions 
>> about a grouping functionality: 
>> https://www.google.com/search?q=php+group+elements+site:stackoverflow.com
> 
> All of the examples I looked at are asking for the first kind of
> grouping, that can be implemented as in your first example.
> In all the examples, if two items are equal, they end up in the same group.
> 
> In your proposed behavior, equal items can end up in distinct groups
> depending on their original position in the source array.
> I don't see any questions or examples that ask for this.

This is correct, although, if the array is sorted initially (and depending on 
which operation and what we want to do), we can still solve the same problem by 
using equality check.

The idea is that `array_group` is more general since it works with operators 
other than `==`, whereas the hashmap approach is only limited to equality check.

A good illustration of this is the increasing subsequences problem, or any 
other problem of similar nature.

Here's some more examples:

1. Use `array_group` to create list of singleton list:
```
$groups = array_group( $arr, function( $p1, $p2 ) {
  return false;
} );
```

(This can also be achieved with `array_map` returning `[ $x ]`)

2. Distinct groups for consecutive positive and negative elements

```
$arr = [-1,2,-3,-4,2,1,2,-3,1,1,2];

$groups = array_group( $arr, function( $p1, $p2 ) {
  return ($p1 > 0) == ($p2 > 0);
} );
```

This produces `[[-1],[2],[-3,-4],[2,1,2],[-3],[1,1,2]]`, so we can easily 
capture the groups of highs/lows for example.

3. Group sentences (similar to `explode`, but still different)

```
$arr = "Hello, PHP. Good to see you.";
$groups = array_group( str_split( $arr ), function( $p1, $p2 ) {
return '.' !== $p1;
} );

$groups = array_map( 'join', $groups );
```

Producing `[ "Hello, PHP.", " Good to see you." ]`.

4. Grouping book sections
```
$book_sections = [ '1.0', '1.1', '1.2', '2.0', '2.1', '3.0', '3.1' ];
$groups = array_group( $book_sections, function( $p1, $p2 ) {
return $p1[0] === $p2[0];
} );
```

Producing `[ [ '1.0', '1.1', '1.2' ], [ '2.0', '2.1'], [ '3.0', '3.1' ] ]`

and so on...

Basically, it's a very general utility :)

Best,

Boro


> 
> -- Andreas
> 
>> 
>>> Your behavior can be implemented in userland like so:
>>> https://3v4l.org/epvHm
>> 
>> Correct, but then again, we can also implement 
>> `array_map`/`array_filter`/etc. in userland :)
>> 
>>> I think you need to make a case as to why the behavior you describe
>>> justifies a native function.
>> 
>> Similar to my previous answer, but also in general - ease of access and also 
>> performance.
>> 
>>> E.g. if you find a lot of public php code that does this kind of grouping.
>>> 
>>> I personally suspect it is not that common.
>>> 
>>> Cheers
>>> Andreas
>>> 
>>> 
>>> On Tue, 30 May 2023 at 17:08, Boro Sitnikovski  wrote:
 
 Hey,
 
 Thanks for the suggestion.
 
 For the previous case in the code, I added these in a Gist to not clutter 
 here too much:
 
 1. The first example corresponds to 
 https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_manual_group-php
 2. The second example corresponds to 
 https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_group-php
 3. Another example, addressing the problem of increasing subsequences is 
 very simple with `array_group`: 
 https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_incr_subseqs-php
 
 Best,
 
 Boro
 
> On 30.5.2023, at 16:57, Andreas Hennings  wrote:
> 
> Hello Boro,
> I think you should include the "expected result" in your code examples.
> Maybe this is in your patch file, but I don't think we want to look at
> that for discussion.
> 
> Cheers
> Andreas
> 
> On Tue, 30 May 2023 at 13:35, Boro Sitnikovski  
> wrote:
>> 
>> Hello all,
>> 
>> As per the How To Create an RFC instructions, I am sending this e-mail 
>> in order to get your feedback on my proposal.
>> 
>> I propose introducing a function to PHP core named `array_group`. This 
>> function takes an array and a function and returns an array that 
>> contains arrays - groups of consecutive elements. This is very similar 
>> to Haskell's `groupBy` function.
>> 
>> For some background as to why - usually, when people want to do grouping 
>> in PHP, they use hash maps, so something like:
>> 
>> ```
>> > $array = [
>> [ 'id' => 1, 'value' => 'foo' ],
>> [ 'id' => 1, 'value' => 'bar' 

Re: [PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Larry Garfield
On Tue, May 30, 2023, at 4:27 PM, Boro Sitnikovski wrote:
> Hi,
>
> Thank you for your thoughts.
>
>> I would say the more common desired behavior is the one in your first
>> example. And even for that we don't have a native function.
>
> This Google search might give more insight into the number of 
> discussions about a grouping functionality: 
> https://www.google.com/search?q=php+group+elements+site:stackoverflow.com
>
>> Your behavior can be implemented in userland like so:
>> https://3v4l.org/epvHm
>
> Correct, but then again, we can also implement 
> `array_map`/`array_filter`/etc. in userland :)
>
>> I think you need to make a case as to why the behavior you describe
>> justifies a native function.
>
> Similar to my previous answer, but also in general - ease of access and 
> also performance.

Do you have benchmarks showing that implementing it in C would be notably 
faster?  That would help the case that it should be written in C.

Also, please do not top-post.

--Larry Garfield

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



Re: [PHP-DEV] [RFC] Property hooks, nee accessors

2023-05-30 Thread Larry Garfield



-- 
  Larry Garfield
  la...@garfieldtech.com

On Mon, May 29, 2023, at 8:28 PM, Claude Pache wrote:
>> Le 8 mai 2023 à 23:38, Larry Garfield  a écrit :
>> 
>> Ilija Tovilo and I would like to offer another RFC for your consideration.  
>> It's been a while in coming, and we've evolved the design quite a bit just 
>> in the last week so if you saw an earlier draft of it in the past few 
>> months, I would encourage you to read it over again to make sure we're all 
>> on the same page.  I'm actually pretty happy with where it ended up, even if 
>> it's not the original design.  This approach eliminates several 
>> hard-to-implement edge cases while still providing a lot of functionality in 
>> one package.
>> 
>> https://wiki.php.net/rfc/property-hooks
>> 
>
> Hi,
>
> If I understand correctly, given:
>
> 
> class C {
>
> public int $someInt;
>
> public float $someFloat;
>
> public int $someIntWithHook {
> get => $field;
> set => $field = $value;
> }
>
> public float $someFloatWithHook {
> get => $field;
> set => $field = $value;
> }
>
> }
> ?>
>
> we have:
>
>  $obj = new C;
> var_dump($obj->someInt = 42.0); // int(42)
> var_dump($obj->someFloat = 42); // float(42)
> ?>
>
> but:
>
>  $obj = new C;
> var_dump($obj->someIntWithHook = 42.0); // float(42)
> var_dump($obj->someFloatWithHook = 42); // int(42)
> ?>
>
> If I am correct, it means that the “This also implies that adding a set 
> hook to a property cannot change the result of the = operator” 
> statement is a bit too optimistic.

We looked into this a bit; it's correct if you're in weak mode.  In strict 
mode, it applies only to $o->float = $anInt, as that's the only legal type 
coercion.  Still, you're right that it's not quite "cannot change", so I've 
adjusted the wording to better describe the edge cases.  The behavior is still 
the same as __set(), so we don't see a need to change it further.

Thanks for the catch.

--Larry Garfield

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



Re: [PHP-DEV] Declaration-aware attributes

2023-05-30 Thread Larry Garfield
On Tue, May 30, 2023, at 2:49 PM, Andreas Hennings wrote:
> On Tue, 30 May 2023 at 15:14, Stephen Reay  wrote:
>>
>> (Resending to the list without all the history because qmail complained 
>> about message size)
>>
>>
>> >>
>> >> Hi Andreas,
>> >>
>> >> I too have wondered (and I think asked in room11?) about such a concept. 
>> >> >From memory the general response was “just do it in userland with a 
>> >> wrapper” so its good to see someone else is interested in this being part 
>> >> of the language.
>> >>
>> >> While I agree that it’s most useful if the `Reflector` instance is 
>> >> available in the constructor, I’m not keen on the proposed magic 
>> >> “skipping” of arguments as you suggest. It seems way too easy to confuse 
>> >> someone (particularly if the attribute class itself has reason to be 
>> >> instantiated directly in code)
>> >
>> > Good point! Almost made me change my mind completely. But I already
>> > changed it back :)
>> >
>> > When instantiating in code, the "real" signature would have to be
>> > used, and the reflector argument passed explicitly.
>>
>>
>> That’s kind of my point: it’s not super intuitive why (or the specifics of 
>> how) it’s being skipped when it’s an attribute, vs when it’s instantiated 
>> from code. What if someone specifies an argument with the same name? If they 
>> specify args without names, can they just use null for that? Etc.
>
> I agree it could be confusing.
> But for the named args, I think it is quite obvious:
>
> #[Attribute]
> class A {
>   public readonly array $moreArgs;
>   public function __construct(
> public readonly string $x,
> // Reflector parameter can be last required parameter, BUT
> #[AttributeDeclaration] public readonly \ReflectionClass $class,
> // Optional parameters have to be after the required reflector parameter.
> public readonly ?string $y = NULL,
> // Variadic parameter must be last.
> string ...$moreArgs,
>   ) {
> $this->moreArgs = $moreArgs;
>   }
> }
>
> #[A('x', 'y', 'z')]  // -> new A('x', $reflector, 'y', 'z')
> #[A(x: 'x', y: 'y')]  // -> new A('x', $reflector, 'y')
> #[A(x: 'x', class: new ReflectionClass('C'))]  // -> new A('x', new
> ReflectionClass('C'))
>
> We _could_ say that explicitly passing a value for the reflector in an
> attribute declaration is forbidden.
> Or we allow it, then an explicit value would simply overwrite the
> implicit value.
>
> If we use placeholder syntax, the above examples would look like this:
>
> #[A('x', ?, 'y', 'z')]  // -> new A('x', $reflector, 'y', 'z')
> #[A(x: 'x', class: ?, y: 'y')]  // -> new A('x', $reflector, 'y')
> #[A(x: 'x', class: new ReflectionClass('C'))]  // -> new A('x', new
> ReflectionClass('C'))
>
>
>>
>> > This would be
>> > useful for unit tests that want to replicate the realistic behavior.
>> > Also it guarantees that the code of the attribute class can really
>> > count on this value to not be null, no matter how the class is
>> > instantiated.
>> >
>> >
>>
>> I would expect that whether the Reflector object is required is simply a 
>> matter of whether or not the parameter is nullable.
>> If it’s not nullable, then yes, the explicit instantiation call will need to 
>> supply it at the correct location. If it’s only required when created from 
>> attribute usage, then it would accept null, and the constructor would have 
>> appropriate logic to handle that.
>>
>
> Yes.
> But I would expect the common practice to be to make it required,
> because then the constructor code will be simpler.
>
>>
>>
>> >>
>> >> I think a better approach would be to suggest authors put the parameter 
>> >> at the *end* of the parameter list, so that no ‘skipping' is required 
>> >> when passing arguments without names (or put it where you like if you’re 
>> >> always using named arguments)
>> >
>> > If I understand correctly, the proposal would technically not change,
>> > we just add a recommendation.
>>
>> Technically, yes “my way” would work fine with the proposal you’ve 
>> suggested, if I choose to always put the parameter marked by 
>> #[ReflectionContext] last.
>
> As above, the problem with this would be optional and variadic
> parameters, which have to come after a required reflector parameter.
>
>>
>> I’m just concerned about confusing usage if “insert this parameter anywhere” 
>> is the ‘recommended’ (i.e. documented example) way to use this feature.
>>
>> Even with that concern, I still prefer this to most other solutions 
>> mentioned so far, for the same reasons: they’re all some degree of magic.
>>
>> The only other solution I can think of that’s less “magic” and more 
>> explicit, is (and I have no idea if this is even feasible technically) to 
>> introduce a builtin trait for attribute classes to use, providing a 
>> protected method or property that gives access to the Reflector (how the 
>> trait has access is not really important, I assume it can be assigned to the 
>> object somehow before the constructor is called). I 

Re: [PHP-DEV] [RFC] [Discussion] Clone with

2023-05-30 Thread Tim Düsterhus

Hi

On 5/30/23 18:37, Larry Garfield wrote:

To be honest, the current behavior seemed like the natural choice for
me, and I didn't really consider to execute the __clone() method after the
clone assignments.
Do you have a use-case in mind when you need to forward-pass information to
__clone()?


Not a specific one off hand.  It's more a conceptual question.  `with` has more 
contextual awareness than __clone(), so it should have "first crack" at the 
operation, so that if necessary it can make changes that __clone() can then respond to.  
The inverse doesn't make sense.

The only reason for `with` to come after would be to allow `with` to "override" or 
"undo" something that __clone() did.  Generally speaking, if you have to undo something 
you just did, you shouldn't have done it in the first place, so that's a less compelling 
combination.

This one isn't a deal breaker, but we should be sure to think it through as 
it's kinda hard to reverse later.


FWIW if I would've create the implementation, I intuitively would also 
have chosen to first execute __clone() and then set the properties. Of 
course this isn't a well-argued reason to do it this way, but I wanted 
to share it nonetheless.


Best regards
Tim Düsterhus

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



Re: [PHP-DEV] Let ReflectionMethod keep track of original class

2023-05-30 Thread Andreas Hennings
On Tue, 30 May 2023 at 18:48, Larry Garfield  wrote:
>
>
>
> --
>   Larry Garfield
>   la...@garfieldtech.com
>
> On Tue, May 30, 2023, at 2:42 AM, Andreas Hennings wrote:
> > Hello list,
> > this proposal will be useful in combination with "Declaration-aware 
> > attributes"
> >
> >
> > Problem
> > 
> > Currently, ReflectionMethod is not aware of the original class, if the
> > method is declared in a parent class.
> > Methods that are called during a discovery algorithm that need to
> > process a method with its original class typically need two
> > parameters:
> >
> > function processMethod(\ReflectionClass $class, \ReflectionMethod $method) 
> > {..}
> >
> >
> > Proposal
> > 
> > Let a ReflectionMethod object keep track of the original class.
> > Introduce a new method ReflectionMethod->getOriginalClass() to retrieve it.
> >
> > class B {
> >   function f($x) {}
> > }
> > class C extends B {}
> >
> > foreach ([
> >   // There are different ways to get a reflection method object, all
> > of them track the original class.
> >   new ReflectionMethod('C', 'f'),
> >   (new ReflectionClass('C'))->getMethod('f'),
> >   (new ReflectionMethod('C', 
> > 'f'))->getParameters()[0]->getDeclaringFunction(),
> > ] as $rm) {
> >   // The following won't change:
> >   assert($rm->class === 'B');
> >   assert($rm->getDeclaringClass()->getName() === 'B');
> >   // New method:
> >   assert($rm->getOriginalClass()->getName() === 'C');
> >
> >
> > Alternatives
> > ==
> >
> > At first I thought we might introduce a new class like
> > "VirtualReflectionMethod" which behaves as if the method was declared
> > on the child class. But this is awkward.
> >
> > I think the ->getOriginalClass() is much simpler.
>
>
> I would not be opposed to this.  I don't know that I have any use cases for 
> it, but I'd be open to it.

You can search in your favourite project's /vendor/ directory for
methods with two or more parameters that receive reflector objects.

"function .*\(.*Reflection\w+ \$\w+, .?Reflection\w+ \$\w+"

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

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



Re: [PHP-DEV] Let ReflectionMethod keep track of original class

2023-05-30 Thread Larry Garfield



-- 
  Larry Garfield
  la...@garfieldtech.com

On Tue, May 30, 2023, at 2:42 AM, Andreas Hennings wrote:
> Hello list,
> this proposal will be useful in combination with "Declaration-aware 
> attributes"
>
>
> Problem
> 
> Currently, ReflectionMethod is not aware of the original class, if the
> method is declared in a parent class.
> Methods that are called during a discovery algorithm that need to
> process a method with its original class typically need two
> parameters:
>
> function processMethod(\ReflectionClass $class, \ReflectionMethod $method) 
> {..}
>
>
> Proposal
> 
> Let a ReflectionMethod object keep track of the original class.
> Introduce a new method ReflectionMethod->getOriginalClass() to retrieve it.
>
> class B {
>   function f($x) {}
> }
> class C extends B {}
>
> foreach ([
>   // There are different ways to get a reflection method object, all
> of them track the original class.
>   new ReflectionMethod('C', 'f'),
>   (new ReflectionClass('C'))->getMethod('f'),
>   (new ReflectionMethod('C', 
> 'f'))->getParameters()[0]->getDeclaringFunction(),
> ] as $rm) {
>   // The following won't change:
>   assert($rm->class === 'B');
>   assert($rm->getDeclaringClass()->getName() === 'B');
>   // New method:
>   assert($rm->getOriginalClass()->getName() === 'C');
>
>
> Alternatives
> ==
>
> At first I thought we might introduce a new class like
> "VirtualReflectionMethod" which behaves as if the method was declared
> on the child class. But this is awkward.
>
> I think the ->getOriginalClass() is much simpler.


I would not be opposed to this.  I don't know that I have any use cases for it, 
but I'd be open to it.

--Larry Garfield

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



Re: [PHP-DEV] [RFC] [Discussion] Clone with

2023-05-30 Thread Larry Garfield
On Mon, May 29, 2023, at 11:22 PM, Máté Kocsis wrote:
> Hi Michał and Larry,
>
> As Tim has already clarified, using literal strings in the left-hand side
> of "clone with expressions" won't cause any issues
> for IDEs and static analysers to identify the correct property. I got rid
> of the shorthand syntax from the proposal because
> it is not strictly required (since it has an equivalent alternative), and I
> acknowledge the fact that there are certain people
> who don't want to have two syntaxes to express the same thing. If it turns
> out that we really want to have the short-hand
> syntax as well, we can always add support for it later.

My point is, rather, that if we want to avoid having multiple syntaxes (which 
is valid), the existing named-args syntax and behavior already more than 
adequately covers all bases.

> That approach, taken now, gives us all the flexibility people have been
>> asking for, reuses the existing named arguments syntax entirely, and is the
>> most refactorable option.
>>
>
> Your suggestion is very interesting/promising... However, I'm a bit worried
> that if I only supported the "identifier: expression" syntax for
> "clone assignments", and I required people to pass an array (or splatted
> array) instead of the "expression => expression" syntax,
> they would start complaining why they cannot directly use expressions as
> property names... That's why I think all the alternatives are useful
> on their own, but only the "expressions => expression" syntax is
> vital, since all the other use-cases can be written like this.

Has the "if you need a variable name/number, use an array" been a problem for 
named arguments?  I've not seen it been one.  Has anyone else?

You cannot use an expression as a function argument name today, and... no one 
seems to mind.  Using an array in that rare edge case is fine.

I think we agree that the most common case by far will be a fixed, small set of 
statically-named properties.  So we should optimize for that case, which is the 
named-args style.

>> Additionally, the current "property names expression" section shows a very
>> odd example.  It's recloning the object N times, reinvoking the __clone
>> method each time, and reassigning a single property.  If you're updating
>> several properties at once, that is extremely inefficient.  Dynamic
>> property names are not the solution there; allowing the list to be dynamic
>> in the first place is, and that should not be punted to future scope.
>>
>
> I'm aware that it's currently inefficient, but it's the case only when you
> want to update a dynamic list of properties. In my opinion, you update
> a fixed set of properties most of the time
> (e.g. Psr\Http\Message\ResponseInterface::withStatus()), and it's only a
> rare case when you need
> more dynamic behavior. That's why I believe it's not a huge problem to keep
> cloning objects unnecessarily until we add support for the stuff
> mentioned in the future scope section.

Except that named-args style gives us the better alternative today, with known, 
pre-existing syntax and semantics.

> I am also confused by the choice to make __clone() run before with with
>> properties.  I would have expected it to be the other way around.  Can you
>> explain (in the RFC) why you did it that way?  That seems more limiting
>> than the alternative, as you cannot forward-pass information to __clone()
>> this way.
>>
>
> To be honest, the current behavior seemed like the natural choice for
> me, and I didn't really consider to execute the __clone() method after the
> clone assignments.
> Do you have a use-case in mind when you need to forward-pass information to
> __clone()?

Not a specific one off hand.  It's more a conceptual question.  `with` has more 
contextual awareness than __clone(), so it should have "first crack" at the 
operation, so that if necessary it can make changes that __clone() can then 
respond to.  The inverse doesn't make sense.

The only reason for `with` to come after would be to allow `with` to "override" 
or "undo" something that __clone() did.  Generally speaking, if you have to 
undo something you just did, you shouldn't have done it in the first place, so 
that's a less compelling combination.

This one isn't a deal breaker, but we should be sure to think it through as 
it's kinda hard to reverse later.

--Larry Garfield

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



Re: [PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Andreas Hennings
On Tue, 30 May 2023 at 18:27, Boro Sitnikovski  wrote:
>
> Hi,
>
> Thank you for your thoughts.
>
> > I would say the more common desired behavior is the one in your first
> > example. And even for that we don't have a native function.
>
> This Google search might give more insight into the number of discussions 
> about a grouping functionality: 
> https://www.google.com/search?q=php+group+elements+site:stackoverflow.com

All of the examples I looked at are asking for the first kind of
grouping, that can be implemented as in your first example.
In all the examples, if two items are equal, they end up in the same group.

In your proposed behavior, equal items can end up in distinct groups
depending on their original position in the source array.
I don't see any questions or examples that ask for this.

-- Andreas

>
> > Your behavior can be implemented in userland like so:
> > https://3v4l.org/epvHm
>
> Correct, but then again, we can also implement 
> `array_map`/`array_filter`/etc. in userland :)
>
> > I think you need to make a case as to why the behavior you describe
> > justifies a native function.
>
> Similar to my previous answer, but also in general - ease of access and also 
> performance.
>
> > E.g. if you find a lot of public php code that does this kind of grouping.
> >
> > I personally suspect it is not that common.
> >
> > Cheers
> > Andreas
> >
> >
> > On Tue, 30 May 2023 at 17:08, Boro Sitnikovski  wrote:
> >>
> >> Hey,
> >>
> >> Thanks for the suggestion.
> >>
> >> For the previous case in the code, I added these in a Gist to not clutter 
> >> here too much:
> >>
> >> 1. The first example corresponds to 
> >> https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_manual_group-php
> >> 2. The second example corresponds to 
> >> https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_group-php
> >> 3. Another example, addressing the problem of increasing subsequences is 
> >> very simple with `array_group`: 
> >> https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_incr_subseqs-php
> >>
> >> Best,
> >>
> >> Boro
> >>
> >>> On 30.5.2023, at 16:57, Andreas Hennings  wrote:
> >>>
> >>> Hello Boro,
> >>> I think you should include the "expected result" in your code examples.
> >>> Maybe this is in your patch file, but I don't think we want to look at
> >>> that for discussion.
> >>>
> >>> Cheers
> >>> Andreas
> >>>
> >>> On Tue, 30 May 2023 at 13:35, Boro Sitnikovski  
> >>> wrote:
> 
>  Hello all,
> 
>  As per the How To Create an RFC instructions, I am sending this e-mail 
>  in order to get your feedback on my proposal.
> 
>  I propose introducing a function to PHP core named `array_group`. This 
>  function takes an array and a function and returns an array that 
>  contains arrays - groups of consecutive elements. This is very similar 
>  to Haskell's `groupBy` function.
> 
>  For some background as to why - usually, when people want to do grouping 
>  in PHP, they use hash maps, so something like:
> 
>  ```
>    $array = [
>  [ 'id' => 1, 'value' => 'foo' ],
>  [ 'id' => 1, 'value' => 'bar' ],
>  [ 'id' => 2, 'value' => 'baz' ],
>  ];
> 
>  $groups = [];
>  foreach ( $array as $element ) {
>    $groups[ $element['id'] ][] = $element;
>  }
> 
>  var_dump( $groups );
>  ```
> 
>  This can now be achieved as follows (not preserving keys):
> 
>  ```
>    $array = [
>  [ 'id' => 1, 'value' => 'foo' ],
>  [ 'id' => 1, 'value' => 'bar' ],
>  [ 'id' => 2, 'value' => 'baz' ],
>  ];
> 
>  $groups = array_group( $array, function( $a, $b ) {
>  return $a['id'] == $b['id'];
>  } );
>  ```
> 
>  The disadvantage of the first approach is that we are only limited to 
>  using equality check, and we cannot group by, say, `<` or other 
>  functions.
>  Similarly, the advantage of the first approach is that the keys are 
>  preserved, and elements needn't be consecutive.
> 
>  In any case, I think a utility function such as `array_group` will be 
>  widely useful.
> 
>  Please find attached a patch with a proposed implementation. Curious 
>  about your feedback.
> 
>  Best,
> 
>  Boro Sitnikovski
> 
> >>
>

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



Re: [PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Boro Sitnikovski
Hi,

Thank you for your thoughts.

> I would say the more common desired behavior is the one in your first
> example. And even for that we don't have a native function.

This Google search might give more insight into the number of discussions about 
a grouping functionality: 
https://www.google.com/search?q=php+group+elements+site:stackoverflow.com

> Your behavior can be implemented in userland like so:
> https://3v4l.org/epvHm

Correct, but then again, we can also implement `array_map`/`array_filter`/etc. 
in userland :)

> I think you need to make a case as to why the behavior you describe
> justifies a native function.

Similar to my previous answer, but also in general - ease of access and also 
performance.

> E.g. if you find a lot of public php code that does this kind of grouping.
> 
> I personally suspect it is not that common.
> 
> Cheers
> Andreas
> 
> 
> On Tue, 30 May 2023 at 17:08, Boro Sitnikovski  wrote:
>> 
>> Hey,
>> 
>> Thanks for the suggestion.
>> 
>> For the previous case in the code, I added these in a Gist to not clutter 
>> here too much:
>> 
>> 1. The first example corresponds to 
>> https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_manual_group-php
>> 2. The second example corresponds to 
>> https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_group-php
>> 3. Another example, addressing the problem of increasing subsequences is 
>> very simple with `array_group`: 
>> https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_incr_subseqs-php
>> 
>> Best,
>> 
>> Boro
>> 
>>> On 30.5.2023, at 16:57, Andreas Hennings  wrote:
>>> 
>>> Hello Boro,
>>> I think you should include the "expected result" in your code examples.
>>> Maybe this is in your patch file, but I don't think we want to look at
>>> that for discussion.
>>> 
>>> Cheers
>>> Andreas
>>> 
>>> On Tue, 30 May 2023 at 13:35, Boro Sitnikovski  wrote:
 
 Hello all,
 
 As per the How To Create an RFC instructions, I am sending this e-mail in 
 order to get your feedback on my proposal.
 
 I propose introducing a function to PHP core named `array_group`. This 
 function takes an array and a function and returns an array that contains 
 arrays - groups of consecutive elements. This is very similar to Haskell's 
 `groupBy` function.
 
 For some background as to why - usually, when people want to do grouping 
 in PHP, they use hash maps, so something like:
 
 ```
 >>> $array = [
 [ 'id' => 1, 'value' => 'foo' ],
 [ 'id' => 1, 'value' => 'bar' ],
 [ 'id' => 2, 'value' => 'baz' ],
 ];
 
 $groups = [];
 foreach ( $array as $element ) {
   $groups[ $element['id'] ][] = $element;
 }
 
 var_dump( $groups );
 ```
 
 This can now be achieved as follows (not preserving keys):
 
 ```
 >>> $array = [
 [ 'id' => 1, 'value' => 'foo' ],
 [ 'id' => 1, 'value' => 'bar' ],
 [ 'id' => 2, 'value' => 'baz' ],
 ];
 
 $groups = array_group( $array, function( $a, $b ) {
 return $a['id'] == $b['id'];
 } );
 ```
 
 The disadvantage of the first approach is that we are only limited to 
 using equality check, and we cannot group by, say, `<` or other functions.
 Similarly, the advantage of the first approach is that the keys are 
 preserved, and elements needn't be consecutive.
 
 In any case, I think a utility function such as `array_group` will be 
 widely useful.
 
 Please find attached a patch with a proposed implementation. Curious about 
 your feedback.
 
 Best,
 
 Boro Sitnikovski
 
>> 

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



Re: [PHP-DEV] Deprecation of the formats DATE_ISO8601 and DATE_RFC7231

2023-05-30 Thread Hans Henrik Bergan
>In my opinion, deprecating this does not do anything besides annoying
users.

In my opinion, since it isn't, and likely never was, a legal ISO8601
string, it's a no-brainer that it should be deprecated. (it's at least
been illegal since iso8601:2004 released in 2004)

On Fri, 26 May 2023 at 12:17, Derick Rethans  wrote:
>
> On Fri, 19 May 2023, Jorg Sowa wrote:
>
> > I would like to propose the deprecation of the constants DATE_ISO8601,
> > DateTimeInterface::ISO8601 and DateTimeInterface::RFC7231, DATE_RFC7231.
>
> …
>
> > In my opinion the question is not whether this constant should be
> > deprecated, but when. I know that the problem was discussed in the past,
> > but I hope enough time has passed already to touch this topic again
> > although of the BC nature. [6] 
> > [7]
>
> In my opinion, deprecating this does not do anything besides annoying
> users.
>
> > 
> > Arguments for deprecating DATE_RFC7231:
> > - error prone nature, the format never really supported timezone. I'm not
> > sure how it appeared in the code, but tests clearly lack cases for this
> > format. I'm not sure if it was missed on purpose, but with tests it would
> > be obvious that the format shouldn't ever appear in the core. [8]
> >  [9]
> > 
>
> This should instead be fixed of being deprecated. It is a recent
> addition after all.
>
> cheers,
> Derick
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php

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



Re: [PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Andreas Hennings
Here we go,
https://3v4l.org/KsL3o


function array_group(array $arr1, callable $compare): array {
$groups = [];
$group = [];
$prev = NULL;
foreach ($arr1 as $value) {
if ($group && !$compare($prev, $value)) {
$groups[] = $group;
$group = [];
}
$group[] = $value;
$prev = $value;
}
if ($group) {
$groups[] = $group;
}
return $groups;
}

On Tue, 30 May 2023 at 18:21, Andreas Hennings  wrote:
>
> Thank you, this clarifies and it confirms my initial assumption of
> what you are proposing.
> So you want to slice an array by comparing adjacent values.
>
>
> My personal feedback:
> I think the need for the grouping behavior you describe is not common
> enough that it needs its own native function.
> I would say the more common desired behavior is the one in your first
> example. And even for that we don't have a native function.
>
> Your behavior can be implemented in userland like so:
> https://3v4l.org/epvHm
>
>
> $arr1 = array(1,2,2,3,1,2,0,4,5,2);
>
> $groups = [];
> $group = [];
> $prev = NULL;
> foreach ($arr1 as $value) {
> if ($group && $prev > $value) {
> $groups[] = $group;
> $group = [];
> }
> $group[] = $value;
> $prev = $value;
> }
> if ($group) {
> $groups[] = $group;
> }
>
> print_r($groups);
>
>
> If needed, the comparison function can be separated out and passed as
> a parameter.
> So the array_group() function with a comparison callback parameter can
> be implemented in userland.
>
> I think you need to make a case as to why the behavior you describe
> justifies a native function.
> E.g. if you find a lot of public php code that does this kind of grouping.
>
> I personally suspect it is not that common.
>
> Cheers
> Andreas
>
>
> On Tue, 30 May 2023 at 17:08, Boro Sitnikovski  wrote:
> >
> > Hey,
> >
> > Thanks for the suggestion.
> >
> > For the previous case in the code, I added these in a Gist to not clutter 
> > here too much:
> >
> > 1. The first example corresponds to 
> > https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_manual_group-php
> > 2. The second example corresponds to 
> > https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_group-php
> > 3. Another example, addressing the problem of increasing subsequences is 
> > very simple with `array_group`: 
> > https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_incr_subseqs-php
> >
> > Best,
> >
> > Boro
> >
> > > On 30.5.2023, at 16:57, Andreas Hennings  wrote:
> > >
> > > Hello Boro,
> > > I think you should include the "expected result" in your code examples.
> > > Maybe this is in your patch file, but I don't think we want to look at
> > > that for discussion.
> > >
> > > Cheers
> > > Andreas
> > >
> > > On Tue, 30 May 2023 at 13:35, Boro Sitnikovski  
> > > wrote:
> > >>
> > >> Hello all,
> > >>
> > >> As per the How To Create an RFC instructions, I am sending this e-mail 
> > >> in order to get your feedback on my proposal.
> > >>
> > >> I propose introducing a function to PHP core named `array_group`. This 
> > >> function takes an array and a function and returns an array that 
> > >> contains arrays - groups of consecutive elements. This is very similar 
> > >> to Haskell's `groupBy` function.
> > >>
> > >> For some background as to why - usually, when people want to do grouping 
> > >> in PHP, they use hash maps, so something like:
> > >>
> > >> ```
> > >>  > >> $array = [
> > >> [ 'id' => 1, 'value' => 'foo' ],
> > >> [ 'id' => 1, 'value' => 'bar' ],
> > >> [ 'id' => 2, 'value' => 'baz' ],
> > >> ];
> > >>
> > >> $groups = [];
> > >> foreach ( $array as $element ) {
> > >>$groups[ $element['id'] ][] = $element;
> > >> }
> > >>
> > >> var_dump( $groups );
> > >> ```
> > >>
> > >> This can now be achieved as follows (not preserving keys):
> > >>
> > >> ```
> > >>  > >> $array = [
> > >> [ 'id' => 1, 'value' => 'foo' ],
> > >> [ 'id' => 1, 'value' => 'bar' ],
> > >> [ 'id' => 2, 'value' => 'baz' ],
> > >> ];
> > >>
> > >> $groups = array_group( $array, function( $a, $b ) {
> > >> return $a['id'] == $b['id'];
> > >> } );
> > >> ```
> > >>
> > >> The disadvantage of the first approach is that we are only limited to 
> > >> using equality check, and we cannot group by, say, `<` or other 
> > >> functions.
> > >> Similarly, the advantage of the first approach is that the keys are 
> > >> preserved, and elements needn't be consecutive.
> > >>
> > >> In any case, I think a utility function such as `array_group` will be 
> > >> widely useful.
> > >>
> > >> Please find attached a patch with a proposed implementation. Curious 
> > >> about your feedback.
> > >>
> > >> Best,
> > >>
> > >> Boro Sitnikovski
> > >>
> >

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



Re: [PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Andreas Hennings
Thank you, this clarifies and it confirms my initial assumption of
what you are proposing.
So you want to slice an array by comparing adjacent values.


My personal feedback:
I think the need for the grouping behavior you describe is not common
enough that it needs its own native function.
I would say the more common desired behavior is the one in your first
example. And even for that we don't have a native function.

Your behavior can be implemented in userland like so:
https://3v4l.org/epvHm


$arr1 = array(1,2,2,3,1,2,0,4,5,2);

$groups = [];
$group = [];
$prev = NULL;
foreach ($arr1 as $value) {
if ($group && $prev > $value) {
$groups[] = $group;
$group = [];
}
$group[] = $value;
$prev = $value;
}
if ($group) {
$groups[] = $group;
}

print_r($groups);


If needed, the comparison function can be separated out and passed as
a parameter.
So the array_group() function with a comparison callback parameter can
be implemented in userland.

I think you need to make a case as to why the behavior you describe
justifies a native function.
E.g. if you find a lot of public php code that does this kind of grouping.

I personally suspect it is not that common.

Cheers
Andreas


On Tue, 30 May 2023 at 17:08, Boro Sitnikovski  wrote:
>
> Hey,
>
> Thanks for the suggestion.
>
> For the previous case in the code, I added these in a Gist to not clutter 
> here too much:
>
> 1. The first example corresponds to 
> https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_manual_group-php
> 2. The second example corresponds to 
> https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_group-php
> 3. Another example, addressing the problem of increasing subsequences is very 
> simple with `array_group`: 
> https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_incr_subseqs-php
>
> Best,
>
> Boro
>
> > On 30.5.2023, at 16:57, Andreas Hennings  wrote:
> >
> > Hello Boro,
> > I think you should include the "expected result" in your code examples.
> > Maybe this is in your patch file, but I don't think we want to look at
> > that for discussion.
> >
> > Cheers
> > Andreas
> >
> > On Tue, 30 May 2023 at 13:35, Boro Sitnikovski  wrote:
> >>
> >> Hello all,
> >>
> >> As per the How To Create an RFC instructions, I am sending this e-mail in 
> >> order to get your feedback on my proposal.
> >>
> >> I propose introducing a function to PHP core named `array_group`. This 
> >> function takes an array and a function and returns an array that contains 
> >> arrays - groups of consecutive elements. This is very similar to Haskell's 
> >> `groupBy` function.
> >>
> >> For some background as to why - usually, when people want to do grouping 
> >> in PHP, they use hash maps, so something like:
> >>
> >> ```
> >>  >> $array = [
> >> [ 'id' => 1, 'value' => 'foo' ],
> >> [ 'id' => 1, 'value' => 'bar' ],
> >> [ 'id' => 2, 'value' => 'baz' ],
> >> ];
> >>
> >> $groups = [];
> >> foreach ( $array as $element ) {
> >>$groups[ $element['id'] ][] = $element;
> >> }
> >>
> >> var_dump( $groups );
> >> ```
> >>
> >> This can now be achieved as follows (not preserving keys):
> >>
> >> ```
> >>  >> $array = [
> >> [ 'id' => 1, 'value' => 'foo' ],
> >> [ 'id' => 1, 'value' => 'bar' ],
> >> [ 'id' => 2, 'value' => 'baz' ],
> >> ];
> >>
> >> $groups = array_group( $array, function( $a, $b ) {
> >> return $a['id'] == $b['id'];
> >> } );
> >> ```
> >>
> >> The disadvantage of the first approach is that we are only limited to 
> >> using equality check, and we cannot group by, say, `<` or other functions.
> >> Similarly, the advantage of the first approach is that the keys are 
> >> preserved, and elements needn't be consecutive.
> >>
> >> In any case, I think a utility function such as `array_group` will be 
> >> widely useful.
> >>
> >> Please find attached a patch with a proposed implementation. Curious about 
> >> your feedback.
> >>
> >> Best,
> >>
> >> Boro Sitnikovski
> >>
>

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



Re: [PHP-DEV] [RFC] [Discussion] PHP 8.3 deprecations

2023-05-30 Thread Tim Düsterhus

Hi

On 5/30/23 17:52, Go Kudo wrote:

  > It should be deprecated with PHP 8.4 at the earliest to give folks at
least

Indeed, I agree that `lcg_value()` should be deprecated at least in PHP 8.4.

However, `lcg_value()` remains a dangerous function. It still has a weak
initial seeding problem (PID, time), not to mention global state. This is
extremely dangerous for workloads on containers where PIDs tend to be
fixed. Perhaps this should be documented at the time of PHP 8.3 release.


As the function is not seedable in userland, we do not need to preserve 
a specific sequence or behavior. Therefore it should be possible to 
replace the seeding to make use of the CSPRNG and fall back to the old 
and insecure seeding if the CSPRNG fails.


For the same reason, the global state is also less of a problem compared 
to mt_rand() and friends.



Because of the above, I have removed my `lcg_value()` deprecation entry
from the RFC. Thanks!

Thanks!

Best regards
Tim Düsterhus

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



Re: [PHP-DEV] [RFC] [Discussion] PHP 8.3 deprecations

2023-05-30 Thread Go Kudo
2023年5月30日(火) 4:49 Tim Düsterhus :

> Hi
>
> On 5/29/23 08:44, Go Kudo wrote:
> > I realized I was about to add the deprecation of `lcg_value()` and forgot
> > to do so, so I added it.
> >
> > https://wiki.php.net/rfc/deprecations_php_8_3#global_combined_lcg
> >
> > As usual, my English is of low quality, so I would appreciate it if you
> > could point out any problems.
>
> I think it's too early for that. Because:
>
> 1. The replacement is only available as of PHP 8.3. Thus there won't be
> a single version where "replacement is available" and "the function does
> not emit deprecation notices" is both true. It should be deprecated with
> PHP 8.4 at the earliest to give folks at least (!) one version to
> cleanly migrate existing code without suppressing any errors / notices /
> deprecations.
>
> 2. It's not seedable, thus the implementation can be switched to use a
> different engine without affecting existing code.
>
> 3. It's not as commonly misused as mt_rand() is. Primarily because the
> possible use-cases are much more rare.
>
> Best regards
> Tim Düsterhus
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
Hi

 > It should be deprecated with PHP 8.4 at the earliest to give folks at
least

Indeed, I agree that `lcg_value()` should be deprecated at least in PHP 8.4.

However, `lcg_value()` remains a dangerous function. It still has a weak
initial seeding problem (PID, time), not to mention global state. This is
extremely dangerous for workloads on containers where PIDs tend to be
fixed. Perhaps this should be documented at the time of PHP 8.3 release.

Because of the above, I have removed my `lcg_value()` deprecation entry
from the RFC. Thanks!

Best Regards,
Go Kudo


Re: [PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Boro Sitnikovski
Hey,

Thanks for the suggestion.

For the previous case in the code, I added these in a Gist to not clutter here 
too much:

1. The first example corresponds to 
https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_manual_group-php
2. The second example corresponds to 
https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_group-php
3. Another example, addressing the problem of increasing subsequences is very 
simple with `array_group`: 
https://gist.github.com/bor0/b5f449bfe85440d96abd933b9f03b310#file-test_array_incr_subseqs-php

Best,

Boro

> On 30.5.2023, at 16:57, Andreas Hennings  wrote:
> 
> Hello Boro,
> I think you should include the "expected result" in your code examples.
> Maybe this is in your patch file, but I don't think we want to look at
> that for discussion.
> 
> Cheers
> Andreas
> 
> On Tue, 30 May 2023 at 13:35, Boro Sitnikovski  wrote:
>> 
>> Hello all,
>> 
>> As per the How To Create an RFC instructions, I am sending this e-mail in 
>> order to get your feedback on my proposal.
>> 
>> I propose introducing a function to PHP core named `array_group`. This 
>> function takes an array and a function and returns an array that contains 
>> arrays - groups of consecutive elements. This is very similar to Haskell's 
>> `groupBy` function.
>> 
>> For some background as to why - usually, when people want to do grouping in 
>> PHP, they use hash maps, so something like:
>> 
>> ```
>> > $array = [
>> [ 'id' => 1, 'value' => 'foo' ],
>> [ 'id' => 1, 'value' => 'bar' ],
>> [ 'id' => 2, 'value' => 'baz' ],
>> ];
>> 
>> $groups = [];
>> foreach ( $array as $element ) {
>>$groups[ $element['id'] ][] = $element;
>> }
>> 
>> var_dump( $groups );
>> ```
>> 
>> This can now be achieved as follows (not preserving keys):
>> 
>> ```
>> > $array = [
>> [ 'id' => 1, 'value' => 'foo' ],
>> [ 'id' => 1, 'value' => 'bar' ],
>> [ 'id' => 2, 'value' => 'baz' ],
>> ];
>> 
>> $groups = array_group( $array, function( $a, $b ) {
>> return $a['id'] == $b['id'];
>> } );
>> ```
>> 
>> The disadvantage of the first approach is that we are only limited to using 
>> equality check, and we cannot group by, say, `<` or other functions.
>> Similarly, the advantage of the first approach is that the keys are 
>> preserved, and elements needn't be consecutive.
>> 
>> In any case, I think a utility function such as `array_group` will be widely 
>> useful.
>> 
>> Please find attached a patch with a proposed implementation. Curious about 
>> your feedback.
>> 
>> Best,
>> 
>> Boro Sitnikovski
>> 

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



Re: [PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Andreas Hennings
Hello Boro,
I think you should include the "expected result" in your code examples.
Maybe this is in your patch file, but I don't think we want to look at
that for discussion.

Cheers
Andreas

On Tue, 30 May 2023 at 13:35, Boro Sitnikovski  wrote:
>
> Hello all,
>
> As per the How To Create an RFC instructions, I am sending this e-mail in 
> order to get your feedback on my proposal.
>
> I propose introducing a function to PHP core named `array_group`. This 
> function takes an array and a function and returns an array that contains 
> arrays - groups of consecutive elements. This is very similar to Haskell's 
> `groupBy` function.
>
> For some background as to why - usually, when people want to do grouping in 
> PHP, they use hash maps, so something like:
>
> ```
>  $array = [
> [ 'id' => 1, 'value' => 'foo' ],
> [ 'id' => 1, 'value' => 'bar' ],
> [ 'id' => 2, 'value' => 'baz' ],
> ];
>
> $groups = [];
> foreach ( $array as $element ) {
> $groups[ $element['id'] ][] = $element;
> }
>
> var_dump( $groups );
> ```
>
> This can now be achieved as follows (not preserving keys):
>
> ```
>  $array = [
> [ 'id' => 1, 'value' => 'foo' ],
> [ 'id' => 1, 'value' => 'bar' ],
> [ 'id' => 2, 'value' => 'baz' ],
> ];
>
> $groups = array_group( $array, function( $a, $b ) {
> return $a['id'] == $b['id'];
> } );
> ```
>
> The disadvantage of the first approach is that we are only limited to using 
> equality check, and we cannot group by, say, `<` or other functions.
> Similarly, the advantage of the first approach is that the keys are 
> preserved, and elements needn't be consecutive.
>
> In any case, I think a utility function such as `array_group` will be widely 
> useful.
>
> Please find attached a patch with a proposed implementation. Curious about 
> your feedback.
>
> Best,
>
> Boro Sitnikovski
>

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



Re: [PHP-DEV] [RFC] [Discussion] PHP 8.3 deprecations

2023-05-30 Thread Tim Düsterhus

Hi

On 5/29/23 17:41, Nikita Popov wrote:

I don't think we should deprecate mt_rand().

There are plenty of use-cases that require neither a seedable (predictable) RNG 
sequence, nor a cryptographically-secure RNG. For those use-cases (and 
especially one-off uses), mt_rand() is perfect, and going through Randomizer is 
an entirely unnecessary complication.

I think I could get on board with deprecating srand/mt_srand to make 
rand/mt_rand non-seedable, directing people who need a seedable RNG to use 
Randomizer, which is much better suited to that use-case. However, we should 
retain rand/mt_rand themselves for non-seeded use-cases.


I disagree. One of the arguments that I made in favor of the removal is 
basically that "defaults" matter:


Developers should be steered to the secure-by-default choice (i.e. the 
CSPRNG), unless they have specific performance or reproduction 
requirements. I believe the current API trio of rand(), mt_rand() and 
random_int() fails at that, because rand() is the "most convenient" 
function. The function name is short and it appears in a plethora of 
existing tutorials.


The CSPRNG should be sufficiently fast for the vast majority of use 
cases. My i5-2430M with Linux 5.4 can handle 1 million random_int(1, 
100) calls in a second, with mt_rand(1, 100) taking 0.25 seconds for 1 
million calls.


The PHP 8.2 OO API very intentionally defaults the \Random\Randomizer to 
the Secure engine for that reason and this recommendation is also part 
of the documentation I wrote for ext/random:


https://www.php.net/manual/en/class.random-engine-secure.php


The Random\Engine\Secure engine is the recommended safe default choice, unless 
the application requires either reproducible sequences or very high performance.



https://www.php.net/manual/en/class.random-engine.php


The Random\Engine\Secure engine that is backed by a CSPRNG is the recommended 
safe default choice, unless the application requires either reproducible 
sequences or very high performance.


-


With srand/mt_srand removed, we also would not have to produce any particular 
sequence, and would be free to switch the internal RNG to something other than 
Mt19937.


In addition to the above reasoning, rand() and mt_rand() are also 
functions with an overloaded signature which are problematic as per:


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

Switching them to a different engine would not do anything about the 
overloaded signature.




I believe making a clean cut is preferable to each of the following:

1. Not changing anything.
2. Taking away seeding (and backing them by a different non-CSPRNG).
3. Making them effectively aliases to random_int().

However I also don't see a strong need for a removal in PHP 9. I am also 
happy with removing them with PHP 10 at the very earliest to give folks 
plenty of runway to migrate to something else on their preferred pace, 
as long as they are not introduced in any new code without proper 
consideration of their downsides.


Best regards
Tim Düsterhus

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



Re: [PHP-DEV] Declaration-aware attributes

2023-05-30 Thread Andreas Hennings
On Tue, 30 May 2023 at 15:14, Stephen Reay  wrote:
>
> (Resending to the list without all the history because qmail complained about 
> message size)
>
>
> >>
> >> Hi Andreas,
> >>
> >> I too have wondered (and I think asked in room11?) about such a concept. 
> >> >From memory the general response was “just do it in userland with a 
> >> wrapper” so its good to see someone else is interested in this being part 
> >> of the language.
> >>
> >> While I agree that it’s most useful if the `Reflector` instance is 
> >> available in the constructor, I’m not keen on the proposed magic 
> >> “skipping” of arguments as you suggest. It seems way too easy to confuse 
> >> someone (particularly if the attribute class itself has reason to be 
> >> instantiated directly in code)
> >
> > Good point! Almost made me change my mind completely. But I already
> > changed it back :)
> >
> > When instantiating in code, the "real" signature would have to be
> > used, and the reflector argument passed explicitly.
>
>
> That’s kind of my point: it’s not super intuitive why (or the specifics of 
> how) it’s being skipped when it’s an attribute, vs when it’s instantiated 
> from code. What if someone specifies an argument with the same name? If they 
> specify args without names, can they just use null for that? Etc.

I agree it could be confusing.
But for the named args, I think it is quite obvious:

#[Attribute]
class A {
  public readonly array $moreArgs;
  public function __construct(
public readonly string $x,
// Reflector parameter can be last required parameter, BUT
#[AttributeDeclaration] public readonly \ReflectionClass $class,
// Optional parameters have to be after the required reflector parameter.
public readonly ?string $y = NULL,
// Variadic parameter must be last.
string ...$moreArgs,
  ) {
$this->moreArgs = $moreArgs;
  }
}

#[A('x', 'y', 'z')]  // -> new A('x', $reflector, 'y', 'z')
#[A(x: 'x', y: 'y')]  // -> new A('x', $reflector, 'y')
#[A(x: 'x', class: new ReflectionClass('C'))]  // -> new A('x', new
ReflectionClass('C'))

We _could_ say that explicitly passing a value for the reflector in an
attribute declaration is forbidden.
Or we allow it, then an explicit value would simply overwrite the
implicit value.

If we use placeholder syntax, the above examples would look like this:

#[A('x', ?, 'y', 'z')]  // -> new A('x', $reflector, 'y', 'z')
#[A(x: 'x', class: ?, y: 'y')]  // -> new A('x', $reflector, 'y')
#[A(x: 'x', class: new ReflectionClass('C'))]  // -> new A('x', new
ReflectionClass('C'))


>
> > This would be
> > useful for unit tests that want to replicate the realistic behavior.
> > Also it guarantees that the code of the attribute class can really
> > count on this value to not be null, no matter how the class is
> > instantiated.
> >
> >
>
> I would expect that whether the Reflector object is required is simply a 
> matter of whether or not the parameter is nullable.
> If it’s not nullable, then yes, the explicit instantiation call will need to 
> supply it at the correct location. If it’s only required when created from 
> attribute usage, then it would accept null, and the constructor would have 
> appropriate logic to handle that.
>

Yes.
But I would expect the common practice to be to make it required,
because then the constructor code will be simpler.

>
>
> >>
> >> I think a better approach would be to suggest authors put the parameter at 
> >> the *end* of the parameter list, so that no ‘skipping' is required when 
> >> passing arguments without names (or put it where you like if you’re always 
> >> using named arguments)
> >
> > If I understand correctly, the proposal would technically not change,
> > we just add a recommendation.
>
> Technically, yes “my way” would work fine with the proposal you’ve suggested, 
> if I choose to always put the parameter marked by #[ReflectionContext] last.

As above, the problem with this would be optional and variadic
parameters, which have to come after a required reflector parameter.

>
> I’m just concerned about confusing usage if “insert this parameter anywhere” 
> is the ‘recommended’ (i.e. documented example) way to use this feature.
>
> Even with that concern, I still prefer this to most other solutions mentioned 
> so far, for the same reasons: they’re all some degree of magic.
>
> The only other solution I can think of that’s less “magic” and more explicit, 
> is (and I have no idea if this is even feasible technically) to introduce a 
> builtin trait for attribute classes to use, providing a protected method or 
> property that gives access to the Reflector (how the trait has access is not 
> really important, I assume it can be assigned to the object somehow before 
> the constructor is called). I guess this could also be an abstract class, but 
> a trait makes it much easier to adopt so that would be my preferred approach.
>
> So something like
>
> trait AttributeReflector {
> protected function getReflector(): 

Re: [PHP-DEV] Declaration-aware attributes

2023-05-30 Thread Stephen Reay
(Resending to the list without all the history because qmail complained about 
message size)


>> 
>> Hi Andreas,
>> 
>> I too have wondered (and I think asked in room11?) about such a concept. 
>> >From memory the general response was “just do it in userland with a 
>> wrapper” so its good to see someone else is interested in this being part of 
>> the language.
>> 
>> While I agree that it’s most useful if the `Reflector` instance is available 
>> in the constructor, I’m not keen on the proposed magic “skipping” of 
>> arguments as you suggest. It seems way too easy to confuse someone 
>> (particularly if the attribute class itself has reason to be instantiated 
>> directly in code)
> 
> Good point! Almost made me change my mind completely. But I already
> changed it back :)
> 
> When instantiating in code, the "real" signature would have to be
> used, and the reflector argument passed explicitly.


That’s kind of my point: it’s not super intuitive why (or the specifics of how) 
it’s being skipped when it’s an attribute, vs when it’s instantiated from code. 
What if someone specifies an argument with the same name? If they specify args 
without names, can they just use null for that? Etc.

> This would be
> useful for unit tests that want to replicate the realistic behavior.
> Also it guarantees that the code of the attribute class can really
> count on this value to not be null, no matter how the class is
> instantiated.
> 
> 

I would expect that whether the Reflector object is required is simply a matter 
of whether or not the parameter is nullable.

If it’s not nullable, then yes, the explicit instantiation call will need to 
supply it at the correct location. If it’s only required when created from 
attribute usage, then it would accept null, and the constructor would have 
appropriate logic to handle that.



>> 
>> I think a better approach would be to suggest authors put the parameter at 
>> the *end* of the parameter list, so that no ‘skipping' is required when 
>> passing arguments without names (or put it where you like if you’re always 
>> using named arguments)
> 
> If I understand correctly, the proposal would technically not change,
> we just add a recommendation.

Technically, yes “my way” would work fine with the proposal you’ve suggested, 
if I choose to always put the parameter marked by #[ReflectionContext] last.

I’m just concerned about confusing usage if “insert this parameter anywhere” is 
the ‘recommended’ (i.e. documented example) way to use this feature.

Even with that concern, I still prefer this to most other solutions mentioned 
so far, for the same reasons: they’re all some degree of magic.

The only other solution I can think of that’s less “magic” and more explicit, 
is (and I have no idea if this is even feasible technically) to introduce a 
builtin trait for attribute classes to use, providing a protected method or 
property that gives access to the Reflector (how the trait has access is not 
really important, I assume it can be assigned to the object somehow before the 
constructor is called). I guess this could also be an abstract class, but a 
trait makes it much easier to adopt so that would be my preferred approach.

So something like

trait AttributeReflector {
protected function getReflector(): \Reflector {
// do internal stuff
}
}

#[Attribute]
class Foo {
Use \AttributeReflector;

public readonly string $name;

function __construct(?string $name = null) {
$this->name = $name ?? $this->getReflector()->name;
}
}

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



[PHP-DEV] Re: [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Boro Sitnikovski
Updated the patch: added a test about increasing subsequences example, and a minor bugfix.

array_group.patch
Description: Binary data
On 30.5.2023, at 13:34, Boro Sitnikovski  wrote:Hello all,As per the How To Create an RFC instructions, I am sending this e-mail in order to get your feedback on my proposal.I propose introducing a function to PHP core named `array_group`. This function takes an array and a function and returns an array that contains arrays - groups of consecutive elements. This is very similar to Haskell's `groupBy` function.For some background as to why - usually, when people want to do grouping in PHP, they use hash maps, so something like:```$array = [	[ 'id' => 1, 'value' => 'foo' ],	[ 'id' => 1, 'value' => 'bar' ],	[ 'id' => 2, 'value' => 'baz' ],];$groups = [];foreach ( $array as $element ) {    $groups[ $element['id'] ][] = $element;}var_dump( $groups );```This can now be achieved as follows (not preserving keys):```$array = [	[ 'id' => 1, 'value' => 'foo' ],	[ 'id' => 1, 'value' => 'bar' ],	[ 'id' => 2, 'value' => 'baz' ],];$groups = array_group( $array, function( $a, $b ) {	return $a['id'] == $b['id'];} );```The disadvantage of the first approach is that we are only limited to using equality check, and we cannot group by, say, `<` or other functions.Similarly, the advantage of the first approach is that the keys are preserved, and elements needn't be consecutive.In any case, I think a utility function such as `array_group` will be widely useful.Please find attached a patch with a proposed implementation. Curious about your feedback.Best,Boro Sitnikovski

Re: [PHP-DEV] RFC [Discussion]: Marking overridden methods (#[\Override])

2023-05-30 Thread Rowan Tommins

On 29/05/2023 19:29, Tim Düsterhus wrote:
I think this is a flawed premise: Any sort of analysis that PHP itself 
performs can also be performed in userland. 



This isn't actually true. There is a lot of dynamic functionality in PHP 
where correctness can't be proven ahead of time, and run-time checks are 
the only way. That's why Hack evolved away from being a superset of PHP 
to being a related but incompatible language, and why languages like 
Dart continue to include run-time type checks despite mandatory 
pre-compilation analysis.




It's part of the language and thus will *always* be available. If it's 
not always available, then folks who switch jobs or contribute to 
various open source projects that made a different choice with regard 
to analyzers will possibly:


- Be unable to rely on a specific check that they found useful in the 
past, because the new SA tool doesn't implement it.
- Be unable to rely on a specific check working as they learned it, 
because the new SA tool implements it differently.



This is a reasonable argument, but it basically comes down to PHP 
Internals becoming the controller of a standardised set of static 
analysis attributes, because a lot of the functionality will still only 
exist in each of those tools.


I don't know how those tools currently collaborate on designing the 
syntax and semantics for extra attributes or docblock annotations, but 
it's not clear that this would be the right place to do it.



As the proposal is a compile time check, the mistake would also be 
immediately apparent just by loading the class (e.g. as part of *any* 
kind of test), it is not necessary to actually execute the method in 
question. So it should be sufficiently visible and usable even in 
codebases that are in a less-than-stellar state.



The reason I'm so skeptical of this feature is that the category of 
mistake the engine would be able to catch seems like it would be rather 
rare. And I really do think it makes a difference that the author of the 
*inheriting* class is the only one that can trigger the check.


For example: if the maintainer of Guzzle decides this check is useful, 
they can add the attribute to appropriate methods in that code base. 
Guzzle itself already has both PHPStan and Psalm configured with 
automated CI checks, and will likely be able to configure them to warn 
if an Override attribute is missing, which the engine can't do. So the 
only benefit to them is standardising the attribute for use with SA tools.


Users of Guzzle won't see any impact from the Guzzle code base adding 
those attributes; they would have to add them to their own code 
separately. If they are not using any kind of static analysis tools, 
they will not be reminded to add them. They will then only see a benefit 
from remembering to add them in the rare case that a parent class 
removes a method they were previously overriding.


It all seems rather a small niche for the engine to care about, unless 
it's the beginning of a larger project to standardise and encourage such 
attributes.


Regards,

--
Rowan Tommins
[IMSoP]

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



Re: [PHP-DEV] Declaration-aware attributes

2023-05-30 Thread Andreas Hennings
On Tue, 30 May 2023 at 05:22, Stephen Reay  wrote:
>
>
>
> > On 30 May 2023, at 07:48, Andreas Hennings  wrote:
> >
> > Hello internals,
> > I am picking up an idea that was mentioned by Benjamin Eberlei in the past.
> > https://externals.io/message/110217#110395
> > (we probably had the idea independently, but Benjamin's is the first
> > post where I see it mentioned in the list)
> >
> > Quite often I found myself writing attribute classes that need to fill
> > some default values or do some validation based on the symbol the
> > attribute is attached to.
> > E.g. a parameter attribute might require a specific type on that
> > parameter, or it might fill a default value based on the parameter
> > name.
> >
> > Currently I see two ways to do this:
> > 1. Do the logic in the code that reads the attribute, instead of the
> > attribute class. This works ok for one-off attribute classes, but it
> > becomes quite unflexible with attribute interfaces, where 3rd parties
> > can provide their own attribute class implementations.
> > 2. Add additional methods to the attribute class that take the symbol
> > reflector as a parameter, like "setReflectionMethod()", or
> > "setReflectionClass()". Or the method in the attribute class that
> > returns the values can have a reflector as a parameter.
> >
> > Both of these are somewhat limited and unpleasant.
> >
> > I want to propose a new way to do this.
> > Get some feedback first, then maybe an RFC.
> >
> > The idea is to mark constructor parameters of the attribute class with
> > a special parameter attribute, to receive the reflector.
> > The other arguments are then shifted to skip the "special" parameter.
> >
> > #[Attribute]
> > class A {
> >  public function __construct(
> >public readonly string $x,
> >#[AttributeContextClass]
> >public readonly \ReflectionClass $class,
> >public readonly string $y,
> >  ) {}
> > }
> >
> > $a = (new ReflectionClass(C::class))->getAttributes()[0]->newInstance();
> > assert($a instanceof A);
> > assert($a->x === 'x');
> > assert($a->class->getName() === 'C');
> > assert($a->y === 'y');
> >
> > Note that for methods, we typically need to know the method reflector
> > _and_ the class reflector, because the method could be defined in a
> > base class.
> >
> > #[Attribute]
> > class AA {
> >  public function __construct(
> >#[AttributeContextClass]
> >public readonly \ReflectionClass $class,
> >#[AttributeContextMethod]
> >public readonly ReflectionMethod $method,
> >  ) {}
> > }
> >
> > class B {
> >  #[AA]
> >  public function f(): void {}
> > }
> >
> > class CC extends B {}
> >
> > $aa = (new ReflectionMethod(CC::class, 
> > 'f))->getAttributes()[0]->newInstance();
> > assert($a->class->getName() === 'CC');
> > assert($a->method->getName() === 'f');
> >
> > ---
> >
> > Notice that the original proposal by Benjamin would use an interface
> > and a setter method, ReflectorAwareAttribute::setReflector().
> >
> > I prefer to use constructor parameters, because I generally prefer if
> > a constructor creates a complete and immutable object.
> >
> > 
> >
> > Thoughts?
> >
> > -- Andreas
> >
> > --
> > PHP Internals - PHP Runtime Development Mailing List
> > To unsubscribe, visit: https://www.php.net/unsub.php
> >
>
> Hi Andreas,
>
> I too have wondered (and I think asked in room11?) about such a concept. 
> >From memory the general response was “just do it in userland with a wrapper” 
> so its good to see someone else is interested in this being part of the 
> language.
>
> While I agree that it’s most useful if the `Reflector` instance is available 
> in the constructor, I’m not keen on the proposed magic “skipping” of 
> arguments as you suggest. It seems way too easy to confuse someone 
> (particularly if the attribute class itself has reason to be instantiated 
> directly in code)

Good point! Almost made me change my mind completely. But I already
changed it back :)

When instantiating in code, the "real" signature would have to be
used, and the reflector argument passed explicitly. This would be
useful for unit tests that want to replicate the realistic behavior.
Also it guarantees that the code of the attribute class can really
count on this value to not be null, no matter how the class is
instantiated.


>
> I think a better approach would be to suggest authors put the parameter at 
> the *end* of the parameter list, so that no ‘skipping' is required when 
> passing arguments without names (or put it where you like if you’re always 
> using named arguments)

If I understand correctly, the proposal would technically not change,
we just add a recommendation.

The only problem I see is with variadic parameters, which naturally
need to be last.



This brings me to another idea: What if instead of the parameter
attribute we use a pseudo constant default value?
Or some other kind of expression that is only valid in the right context.

#[Attribute]
class A {
  function __construct(
..

[PHP-DEV] [RFC] [Discussion] Add new function `array_group`

2023-05-30 Thread Boro Sitnikovski
Hello all,As per the How To Create an RFC instructions, I am sending this e-mail in order to get your feedback on my proposal.I propose introducing a function to PHP core named `array_group`. This function takes an array and a function and returns an array that contains arrays - groups of consecutive elements. This is very similar to Haskell's `groupBy` function.For some background as to why - usually, when people want to do grouping in PHP, they use hash maps, so something like:```$array = [	[ 'id' => 1, 'value' => 'foo' ],	[ 'id' => 1, 'value' => 'bar' ],	[ 'id' => 2, 'value' => 'baz' ],];$groups = [];foreach ( $array as $element ) {    $groups[ $element['id'] ][] = $element;}var_dump( $groups );```This can now be achieved as follows (not preserving keys):```$array = [	[ 'id' => 1, 'value' => 'foo' ],	[ 'id' => 1, 'value' => 'bar' ],	[ 'id' => 2, 'value' => 'baz' ],];$groups = array_group( $array, function( $a, $b ) {	return $a['id'] == $b['id'];} );```The disadvantage of the first approach is that we are only limited to using equality check, and we cannot group by, say, `<` or other functions.Similarly, the advantage of the first approach is that the keys are preserved, and elements needn't be consecutive.In any case, I think a utility function such as `array_group` will be widely useful.Please find attached a patch with a proposed implementation. Curious about your feedback.Best,Boro Sitnikovski

array_group.patch
Description: Binary data


Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

2023-05-30 Thread Claude Pache
Hi Máté,

> Le 30 mai 2023 à 01:41, Máté Kocsis  a écrit :
> 
> Hi Claude,
> 
>> The replacement methods for IntlCalendar::set() (namely 
>> IntlCalendar::setDate() and IntlCalendar::setDateTime()) must not have a 
>> return type of `void`, but `true`, like the original method, for the two 
>> following reasons:
>> 
>> 1. By changing the returned value, you are introducing an unnecessary hazard 
>> when migrating code.
>> 
>> 2. Per https://www.php.net/IntlCalendar, all modification methods of that 
>> class (clear(), roll(), setTime(), etc.) consistently return `true` on 
>> success and `false` on failure, even when the method is infallible (and thus 
>> would always return `true`). Don’t break consistency.
> 
> 1.  I don't see much risk with this change, since we clearly indicate at the 
> very beginning that the new methods return void, so programmers migrating 
> their code
> should pay attention to the return types. IDEs and static analysers can 
> surely warn them in case of inattention.

The very risk is that the programmer must be aware that the return convention 
has changed. Recall that not everyone run static analysers, especially for such 
a trivial migration, and that not every code is statically analysable (e.g.. 
`$foo->$bar()`).

Of course, the benefice of the change might be worth the risk; but that leads 
to the second point:

> 
> 2. Some of the methods you mentioned have a "true" return type for a few 
> weeks now. The biggest motivation for introducing these was to at least have 
> some chance to
> make them void in the future. Doing so is a risky move indeed, so should be 
> carried out very carefully, if it's possible at all... That's why I consider 
> it beneficial to spare the
> effort and start with a clean slate by adding the new methods in question 
> with their correct return type.

Concretely, if you change the return value (from `true` to `void/null`), you 
will break the following pattern:

setBar(...)) {
// error handling (might be dead code, it doesn’t matter)
}
?>

Any change that introduce a BC break or a migration burden must be justified. 
(There has been enough rant around justified BC breaks, so let’s not introduce 
unjustified ones.)

What is the purpose of changing the return convention of IntlCalendar methods, 
and is it worth?

—Claude