Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119397
MIME-Version: 1.0
References: <8f530a4a-bf48-4259-877e-16ebf8082936@app.fastmail.com>
 <88a59c46-9261-43f1-874d-ffc2f9caff32@app.fastmail.com> <99bc38dc-3d62-4a1e-a99c-e5daab3d779f@app.fastmail.com>
In-Reply-To: <99bc38dc-3d62-4a1e-a99c-e5daab3d779f@app.fastmail.com>
Date: Mon, 23 Jan 2023 17:08:08 +0100
Message-ID: <CAPzBOBM8v9V=Rthz8mJ1rUBhjDUwiDzf=GLoZAJtTzMgwAOPKA@mail.gmail.com>
To: Ollie Read <php@ollie.codes>
Cc: =?UTF-8?Q?Bj=C3=B6rn_Larsson?= <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Introduce the abiltiy to use the first-call-callable
 syntax on non-static methods, statically
From: landers.robert@gmail.com (Robert Landers)

On Mon, Jan 23, 2023 at 4:39 PM Ollie Read <php@ollie.codes> wrote:
>
> You are absolutely correct. I guess the solution would be to handle it di=
fferently in this case.
>
> Creating a closure from a static method would be fine, as it creates a st=
atic closure, but when attempting to create a static closure from a non-sta=
tic method, it would instead return a closure that errors if it isn't bound=
 to an appropriate object. You'd most likely want to restrict this to publi=
c methods only, which would help with the security issues.
>
> There's already a check there that throws an error, so we can already tel=
l the difference there, but the tricky part will be in the returned closure=
. Perhaps something like "BindingClosure" that throws the static error when=
 attempting to call it unbound, or better yet, a more descriptive error abo=
ut it requiring binding.
>
> Would that be feasible?
>
> On Sun, Jan 22, 2023, at 8:36 PM, Larry Garfield wrote:
> > On Sun, Jan 22, 2023, at 11:45 AM, Ollie Read wrote:
> > > Hello all,
> > >
> > > I've created a feature request issue on GitHub (here:
> > > https://github.com/php/php-src/issues/10414), but I have been advised
> > > that it's best to post here.
> > >
> > > What I would like to introduce/suggest, is the ability to create a
> > > closure from a method using the first-class-callable syntax (eg:
> > > MyClass::aMethod(...)), for a non-static method, statically.
> > >
> > > Currently, the following code causes an error.
> > >
> > > ```
> > > class Test {
> > >     public function test(): string { return 'test'; }
> > > }
> > >
> > > $closure =3D Test::test(...);
> > > ```
> > >
> > > I understand why the error is thrown, but, and I'm unsure of the
> > > specifics regarding this, I think we could delay the error until the
> > > closure was called. The reason for this, is that closures can be boun=
d,
> > > so if you followed on from the code above, you could do the following=
:
> > >
> > > ```
> > > $closure->bindTo(new Test);
> > > $closure();
> > > ```
> > >
> > > The above would bind the closure in $closure to the scope of an objec=
t,
> > > which in this case, is the class that the method belongs to.
> > >
> > > The best example I can think, for this, would be when filter a
> > > collection of instances. If you were using a collection library, you
> > > would currently have something like the following:
> > >
> > > ```
> > > $collection->filter(function (Str $string) {
> > >     return !$string->empty();
> > > });
> > > ```
> > >
> > > Whereas it would be much nicer to have the following:
> > >
> > > ```
> > > $collection->filter(Str::empty(...));
> > > ```
> > >
> > > In this situation, the collection library would be responsible for
> > > binding the closure to the value it is iterating.
> >
> > So you'd implement this yourself elsewhere?
> >
> > class Str {
> >   public function empty(): bool { ... }
> > }
> >
> > I don't see in this example how this is any better than what is already=
 currently possible:
> >
> > class Str {
> >   public static function empty(Str $s): bool { ... }
> > }
> >
> > $collection->filter(Str::empty(...));
> >
> > > I have limited experience with PHPs source, and C in general, but my
> > > understanding would be that if we were creating a closure, we would
> > > skip the check for the static method. The code responsible for handli=
ng
> > > the closure call would most require some additional functionality to
> > > check if it was bound to a valid instance, returning an error if it
> > > isn't, and then returning an error if it isn't bound at all and the
> > > method isn't static.
> > >
> > > The more I think about it, the more I think this may require a new ty=
pe
> > > of Closure, or at least a runtime applied interface, to help develope=
rs
> > > determine whether a closure was created using first-class-callable
> > > syntax.
> >
> > This is, I think, the important part here, and would be a prerequisite.=
  Right now there's no way (as far as I know) to differentiate a closure th=
at is callable from one that would be callable if it were bound to an objec=
t.  That's generally not a huge deal in practice as unbound closures are no=
t often used, but what you're suggesting would make them much more likely. =
 Also, a static closure cannot be bound, so you cannot just blindly bind wh=
atever callable you're passed to $this, in your example.  (Besides, blindly=
 binding a closure to $this sounds like a great security hole.)
> >
> > So for some variant of this to work, I think you'd first need to think =
through how to (easily and without dipping into reflection) determine if a =
closure object is bindable (static or not) and if it's already bound.  Once=
 that's figured out, then we can see what, if any, short-hand way to make a=
 not-yet-bound closure makes sense.  (Which could be FCC syntax or not, I d=
on't know.)
> >
> > --Larry Garfield
> >
> > --
> > PHP Internals - PHP Runtime Development Mailing List
> > To unsubscribe, visit: https://www.php.net/unsub.php
> >
> >
>
> ---
> Best Regards,
> *Ollie Read*

FWIW, I think we can "throw out" any automatic binding, that just
complicates things.

In my mind, Test::Func(...) should be treated the same as ['Test',
'Func'] or 'Test::Func' until it is called and if some fancy framework
wants to do something special with the closure, it can do so. FWIW, I
didn't even know this syntax checked anything until today. I can think
of a number of cases where delaying the error to actual execution is
beneficial.

1. Just because the method/object doesn't exist, doesn't mean it won't
exist by the time it is called -- this is PHP after all.
2. I can have a file with 100 million of these things without
triggering any autoloading.
3. PHPStorm currently doesn't trigger this syntax as an error.