Everything

php

First class callable syntax for instance methods

php.internals

[ ]

3 years ago

Hello, internals! I just want to share some thoughts with you regarding
what could be improved in the first class callable syntax.

It is already possible to create Callable from a static method:

```
class Foo {
public static function staticmethod() {}
}

$c = Foo::staticmethod(...);
```

It would be a great pleasure to have the ability to wrap the instance
method the same way. One particular use case for this could be when there's
an array of items and it is necessary to call some getter-method on all of
them.

```
class Verification
{
public function __construct(private string $id) {}

public function getId(): string
{
return $this->id;
}
}

$items = [new Verification('1-2-3-4')];

array_map(Verification::getId(...), $items);

// previous line is an equivalent of this
array_map(static fn (Verification $v) => $v->getId(), $items);
```

Currently this syntax fails in run-time rather than compile-time.

```
PHP Fatal error:  Uncaught Error: Non-static method Verification::getId()
cannot be called statically in /opt/scratches/scratch_439.php:21
```

Please, let me know what you think about it.

Best regards, Yeven

Robert Landers

3 years ago

On Thu, Apr 13, 2023 at 6:00 AM Eugene Sidelnyk <zsidelnik@gmail.com> wrote:

>
> Hello, internals! I just want to share some thoughts with you regarding
> what could be improved in the first class callable syntax.
>
> It is already possible to create Callable from a static method:
>
> ```
> class Foo {
> public static function staticmethod() {}
> }
>
> $c = Foo::staticmethod(...);
> ```
>
> It would be a great pleasure to have the ability to wrap the instance
> method the same way. One particular use case for this could be when there's
> an array of items and it is necessary to call some getter-method on all of
> them.
>
> ```
> class Verification
> {
> public function __construct(private string $id) {}
>
> public function getId(): string
> {
> return $this->id;
> }
> }
>
> $items = [new Verification('1-2-3-4')];
>
> array_map(Verification::getId(...), $items);
>
> // previous line is an equivalent of this
> array_map(static fn (Verification $v) => $v->getId(), $items);
> ```
>
> Currently this syntax fails in run-time rather than compile-time.
>
> ```
> PHP Fatal error:  Uncaught Error: Non-static method Verification::getId()
> cannot be called statically in /opt/scratches/scratch_439.php:21
> ```
>
> Please, let me know what you think about it.
>
> Best regards, Yeven

Hello,

This has been brought up a couple of times, but I can't seem to find
it. I don't think something like this is possible with the current
implementation of first-class-callables (it would need a major
refactor). Currently adding the callable bit like `func(...)` is
roughly the same as just writing a string `'func'` and passing that
around (IIRC, that's literally what it gets turned to after parsing).
The only difference over using a string, is that there is some
validation to check that it is actually a function you can call.

Basically, there's currently no way to do something like what you
suggest in the engine, which makes it quite a big feature. That being
said, we shouldn't be afraid of doing hard things, but it would be a
lot of work for saving just a few characters of typing every now and
then.

Perhaps it would be easier after partial application could be
implemented (latest discussion here:
https://externals.io/message/119678#119678), or maybe someone else has
some better ideas or thoughts.

Cheers,

Rob Landers
Utrecht, Netherlands

Dan Ackroyd

3 years ago

On Thu, 13 Apr 2023 at 07:30, Robert Landers <landers.robert@gmail.com> wrote:

>
> This has been brought up a couple of times, but I can't seem to find
> it.

https://externals.io/message/119392
https://externals.io/message/120011

> I don't think something like this is possible with the current
> implementation of first-class-callables (it would need a major
> refactor).

It's not currently possible, but it wouldn't need a huge refactor...we
could just do what Java does:

https://www.baeldung.com/java-method-references
https://docs.oracle.com/javase/tutorial/java/javaOO/methodreferences.html

// If we copied the Java way of doing it, this:
// $fn = Verification::getId(...);

// $fn would be equivalent to:
$fn = fn (Verification $v) => $v->getId();


And if the method of the class has parameters, those would be after
the first instance parameter:

class Foo {
function bar(int $x) {}
}

$fn = Foo::bar(...);

// Would be equivalent to:
$fn =  fn (Foo $f, int $x) => $f->bar($x);

I can't see a technical reason not to do it like this, but some people
seem to be having negative gut reactions to it, and it's hard to
persuade someone about aesthetics.

Notes here: https://phpopendocs.com/rfc_codex/class_method_callable

cheers
Dan
Ack

Robert Landers

3 years ago

On Thu, Apr 13, 2023 at 3:47 PM Dan Ackroyd <Danack@basereality.com> wrote:

> I can't see a technical reason not to do it like this, but some people
> seem to be having negative gut reactions to it, and it's hard to
> persuade someone about aesthetics.
>
> Notes here: https://phpopendocs.com/rfc_codex/class_method_callable
>
> cheers
> Dan
> Ack

I'd be down for implementing this in a heartbeat if we can agree on a
syntax. I've run into this issue several times and have thought about
it, just never really done anything about it.

Hmm, yeah, looking at the implementation again, it may not be *that*
big of a change. If I'm understanding correctly, it *might* need a new
op-code (but could be done using the existing ZEND_CALLABLE_CONVERT),
but otherwise should be rather straightforward. The bigger changes are
going to be in the Callable implementation. As mentioned, this 'view
of the world' isn't really supported by the engine. You can't perform
this operation (calling a method on an interchangeable object) without
another closure (to my knowledge) to do so.

Perhaps we could sidestep the syntax issue by taking some inspiration
from Rowan(?) and just implement this as a method on \Closure or
\Callable, maybe something like this:

$a = Closure::fromCallable(static fn () => $this->getId());
$b = Closure::bindMany($items); // executes `bind` on each item in the
array, returning an array of closures
foreach($b as $callback) {
$callback();
}

or something like that. My biggest thing is that there's no real way
to save much on typing this out. Unless you are doing something fairly
exotic, you only save a few characters. That's why I said its a lot of
work for not much benefit. I could be totally wrong though because I,
personally, do run into wanting this feature several times a year.

Zoltán Fekete

3 years ago

Hey guys!

> On 13. Apr 2023, at 16:28, Robert Landers <landers.robert@gmail.com> wrote:
>
> I'd be down for implementing this in a heartbeat if we
>
> $a = Closure::fromCallable(static fn () => $this->getId());
> $b = Closure::bindMany($items); // executes `bind` on each item in the
> array, returning an array of closures
> foreach($b as $callback) {
> $callback();
> }

Personally I find it a bit inconvenient, and also: won’t in this case all the closure instance be created so space allocated for them?

> or something like that. My biggest thing is that there's no real way
> to save much on typing this out. Unless you are doing something fairly
> exotic, you only save a few characters. That's why I said its a lot of
> work for not much benefit. I could be totally wrong though because I,
> personally, do run into wanting this feature several times a year.

I can only tell my opinion as I also came across the same case / problem. But what if I do the following:

```
$items = [new Verification(“1”), new Verification(“2”))];

$ids = array_map(NotVerification::getId(…), $items); // ofc I did that so my fault but not allowing something to be done… I think it’s half win. But it’s very misleading
```

Additionally: if I don’t have the class imported and don’t even need it, than it has to be.

Essentially what we only need in the first parameter is the method name that should be called.

I cannot really assess how big of a hassle it would be, but what I personally think that it look nice is something like this:

```
class Verification
{
public function __construct(private string $id) {}

public function getId(): string
{
return $this->id;
}

public static function getIdAbs(self $instance) {
return abs($instance->id);
}
}

$items = [new Verification(“1”), new Verification(“2”))];

array_map($$->getId(…), $items);

array_map($$::getIdAbs(…), $items);
```

This way the class does not have to be imported. Later one might be able to add additional parameters even.

```
class Verification
{
// […]

public static function getIdAbs(self $instance, array $idMap) {
// do something with the $idMap
return abs($instance->id);
}
}

// […]

array_map($$::getIdAbs(…, $idMap), $items);

```

I know this might be a little more exotic. But it is very straight forward for static analysis tools. And no import has to be done for classes.

Zoltán

Zoltán Fekete

3 years ago

> I cannot really assess how big of a hassle it would be, but what I personally think that it look nice is something like this:
>
> ```
> class Verification
> {
>   public function __construct(private string $id) {}
>
>   public function getId(): string
>   {
>       return $this->id;
>   }
>
>   public static function getIdAbs(self $instance) {
>       return abs($instance->id);
>   }
> }
>
> $items = [new Verification(“1”), new Verification(“2”))];
>
> array_map($$->getId(…), $items);
>
> array_map($$::getIdAbs(…), $items);
> ```
>
> This way the class does not have to be imported. Later one might be able to add additional parameters even.
>
> ```
> class Verification
> {
>   // […]
>
>   public static function getIdAbs(self $instance, array $idMap) {
>       // do something with the $idMap
>       return abs($instance->id);
>   }
> }
>
> // […]
>
> array_map($$::getIdAbs(…, $idMap), $items);
>
> ```
>
> I know this might be a little more exotic. But it is very straight forward for static analysis tools. And no import has to be done for classes.

So right after I sent this, I just realised how significantly different it is all I just wrote. But actually using it with the array_map too I think. Because the closure created and the passed to the array_map has no relation to the items in the array passed as a second parameter.

Larry Garfield

3 years ago

On Thu, Apr 13, 2023, at 3:04 PM, Zoltán Fekete wrote:

> Hey guys!
>
>> On 13. Apr 2023, at 16:28, Robert Landers <landers.robert@gmail.com> wrote:
>>
>> I'd be down for implementing this in a heartbeat if we
>>
>> $a = Closure::fromCallable(static fn () => $this->getId());
>> $b = Closure::bindMany($items); // executes `bind` on each item in the
>> array, returning an array of closures
>> foreach($b as $callback) {
>> $callback();
>> }
>
> Personally I find it a bit inconvenient, and also: won’t in this case
> all the closure instance be created so space allocated for them?
>
>> or something like that. My biggest thing is that there's no real way
>> to save much on typing this out. Unless you are doing something fairly
>> exotic, you only save a few characters. That's why I said its a lot of
>> work for not much benefit. I could be totally wrong though because I,
>> personally, do run into wanting this feature several times a year.
>
> I can only tell my opinion as I also came across the same case /
> problem. But what if I do the following:
>
> ```
> $items = [new Verification(“1”), new Verification(“2”))];
>
> $ids = array_map(NotVerification::getId(…), $items); // ofc I did that
> so my fault but not allowing something to be done… I think it’s half
> win. But it’s very misleading
> ```
>
> Additionally: if I don’t have the class imported and don’t even need
> it, than it has to be.
>
> Essentially what we only need in the first parameter is the method name
> that should be called.
>
> I cannot really assess how big of a hassle it would be, but what I
> personally think that it look nice is something like this:
>
> ```
> class Verification
> {
>    public function __construct(private string $id) {}
>
>    public function getId(): string
>    {
>        return $this->id;
>    }
>
>    public static function getIdAbs(self $instance) {
>        return abs($instance->id);
>    }
> }
>
> $items = [new Verification(“1”), new Verification(“2”))];
>
> array_map($$->getId(…), $items);
>
> array_map($$::getIdAbs(…), $items);
> ```
>
> This way the class does not have to be imported. Later one might be
> able to add additional parameters even.
>
> ```
> class Verification
> {
>    // […]
>
>    public static function getIdAbs(self $instance, array $idMap) {
>        // do something with the $idMap
>        return abs($instance->id);
>    }
> }
>
> // […]
>
> array_map($$::getIdAbs(…, $idMap), $items);
>
> ```
>
> I know this might be a little more exotic. But it is very straight
> forward for static analysis tools. And no import has to be done for
> classes.

I believe the fancy academic name for what we're discussing here is "lenses", and I agree that they'd be very useful.  I currently use a user-space wrapper like so:

function prop(string $prop): \Closure
{
return static fn (object $o): mixed => $o->$prop;
}

function method(string $method, ...$args): \Closure
{
return static fn (object $o): mixed => $o->$method(...$args);
}

Which lets me do this:

$parentNames = pipe($vals,
amap(method('getParent')),
amap(prop('name')),
);

But I really don't like the stringy method and property names.  The T_BLING (that is the only name I will allow for $$) marker has been discussed on and off a few times over the years, I think first in Sara's original pipes proposal many years ago.  I was actually going to suggest the same in this thread before I saw you did. :-)

More abstractly, $$ meaning "the only obvious value in context" has potential, but also risk, because "obvious" is not always obvious.  I do think it's an area to explore, though, and would definitely help with functional style code that relies heavily on closures.

--Larry Garfield

Zoltán Fekete

3 years ago

>
> I believe the fancy academic name for what we're discussing here is
> "lenses", and I agree that they'd be very useful.  I currently use a
> user-space wrapper like so:
>
> function prop(string $prop): \Closure
> {
>     return static fn (object $o): mixed => $o->$prop;
> }
>
> function method(string $method, ...$args): \Closure
> {
>     return static fn (object $o): mixed => $o->$method(...$args);
> }
>
> Which lets me do this:
>
> $parentNames = pipe($vals,
>   amap(method('getParent')),
>   amap(prop('name')),
> );

But I really don't like the stringy method and property names.

That sure does it, but not that type safe though.

The T_BLING (that is the only name I will allow for $$) marker has been

> discussed on and off a few times over the years, I think first in Sara's
> original pipes proposal many years ago.  I was actually going to suggest
> the same in this thread before I saw you did. :-)
>

Yes I found it. Actually there were many discussions about it. I didn't
really went through them, but how had it never made through? (I'll take
time to read through ofc)

> More abstractly, $$ meaning "the only obvious value in context" has
> potential, but also risk, because "obvious" is not always obvious.  I do
> think it's an area to explore, though, and would definitely help with
> functional style code that relies heavily on closures.
>

I'm not sure I understand how this could not be "always obvious". IF, -
let's say we stick to what hacklang has:
https://docs.hhvm.com/hack/expressions-and-operators/pipe - I think it's
farily straightforward

$a = $obj->getConfig()
|> array_map($foo => $foo->getId(), $$)
|> array_filter($foo => $foo !== "woof", $$)
|> $this->toResponse($bar, $$, $baz);

But to get back to the original topic: I think this is what you were
looking for.

Zoltán Fekete

Larry Garfield

3 years ago

On Fri, Apr 14, 2023, at 5:52 AM, Zoltán Fekete wrote:

> That sure does it, but not that type safe though.

Right, which is why I'm fully on board with finding a native syntax way of doing it.  (It would be faster, to, without two extra user space function calls.)

> The T_BLING (that is the only name I will allow for $$) marker has been
>> discussed on and off a few times over the years, I think first in Sara's
>> original pipes proposal many years ago.  I was actually going to suggest
>> the same in this thread before I saw you did. :-)
>>
>
> Yes I found it. Actually there were many discussions about it. I didn't
> really went through them, but how had it never made through? (I'll take
> time to read through ofc)

Sara didn't take her version to a vote.  About 2 years ago I tried with a reduced scope version (just the pipe operator, working on callables; partial application was a separate RFC), but it didn't pass.  I want to try again at some point, if I can get assistance on a better implementation than my paltry skills were able to manage before.  (And if we can collectively make a stronger argument for it.)

>> More abstractly, $$ meaning "the only obvious value in context" has
>> potential, but also risk, because "obvious" is not always obvious.  I do
>> think it's an area to explore, though, and would definitely help with
>> functional style code that relies heavily on closures.
>
> I'm not sure I understand how this could not be "always obvious". IF, -
> let's say we stick to what hacklang has:
> https://docs.hhvm.com/hack/expressions-and-operators/pipe - I think it's
> farily straightforward
>
> $a = $obj->getConfig()
>  |> array_map($foo => $foo->getId(), $$)
>  |> array_filter($foo => $foo !== "woof", $$)
>  |> $this->toResponse($bar, $$, $baz);
>
> But to get back to the original topic: I think this is what you were
> looking for.

That's actually a perfect example of where obvious isn't obvious.

Consider, if you instead did:

$a = $obj->getConfig()
|> array_map($$->getId(), $$)

That is, using $$ to refer to "the value passed from the previous pipe" and "make a closure whose argument is an object that we can then operate on".  Both of those are "obvious" users of $$, but when combined... it's confusing to me which $$ is which, at least.

That's one of the reasons I preferred partial function applications to have their own separate syntax and RFC, and have pipes work on just closures/callables directly.  It neatly sidesteps this issue, and leaves $$ free for the "closure that takes an object" syntax.  So the above would become something like:

$a = $obj->getConfig()
|> array_map($$->getId(), ?)

Which is less ambiguous.

in my mind, these are all separate language features (PFA, pipes, and lenses) that have their own distinct uses, but they need to be designed in tandem so that they mix well together.  Sadly, PHP doesn't do well with that kind of mini-roadmap. :-(

--Larry Garfield

Zoltán Fekete

3 years ago

> I can get assistance on a better implementation than my paltry skills were able to manage before.  (And if we can collectively make a stronger argument for it.)

Well sadly that definitely cannot be me. As I just started to get on
board with internal development. But I am more than excited and ready
to help wherever I can.

> $a = $obj->getConfig()
>  |> array_map($$->getId(), $$)
> That is, using $$ to refer to "the value passed from the previous pipe" and "make a closure whose argument is an object that we can then operate on".  Both of those are "obvious" users of $$, but when combined... it's confusing to me which $$ is which, at least.

Yes, I mixed up stuff.

> That's one of the reasons I preferred partial function applications to have their own separate syntax and RFC, and have pipes work on just closures/callables directly.  It neatly sidesteps this issue, and leaves $$ free for the "closure that takes an object" syntax.
> So the above would become something like:
> $a = $obj->getConfig()
>  |> array_map($$->getId(), ?)
>
> Which is less ambiguous.

What if it's not an object? Let's say an array of arrays? Or an array
of value objects with public properties, and I don't want to call
->getId(), instead just ->id?

```
$a = $obj->getConfig()
|> array_map($$["id"], ?)
```

Personally I would keep the $$ for the pipe. Why:
1. Hacklang has it that way.
2. I think a lot of people already associate it with this purpose.
3. I feel that $$ more has “something previously” or “something same”.
As it would be passed through the pipe I feel it fits more.
4. Maybe the following example is silly but

```
$foo = “stuff”;
$baz = “foo”;

$items = getConfig()
|> array_map($$[$$baz], ?) // what is $$baz in this case?

var_dump($items); // what would this be?
```

Sure this could be done if the T_BLING is restricted for the pipe
operator. But I feel, having it restricted there leaves less
opportunity for sloppy code.

Anyways, as you just wrote, these are all individual, separate
language features, but still have to be designed together. So to
sum-up, I would keep the T_BLING for the pipe operator. And how do you
feel about any of the following?

```
$items = array_map(array $value => $value->getId(), $items); // A
shorter anonymous function shorthand

// As a variable all would lead to syntax error, but here as -
whatever we name it - it would work
// Just $
$items = array_map(User $->getId(), $items);
$items = array_map(array $["id"], $items);
// Or $@ / $:
$items = array_map(User $@->getId(), $item);
```

These could work well with pipe operators.

Larry Garfield

3 years ago

On Mon, Apr 17, 2023, at 2:52 PM, Zoltán Fekete wrote:

>> $a = $obj->getConfig()
>>  |> array_map($$->getId(), $$)
>> That is, using $$ to refer to "the value passed from the previous pipe" and "make a closure whose argument is an object that we can then operate on".  Both of those are "obvious" users of $$, but when combined... it's confusing to me which $$ is which, at least.
>
> Yes, I mixed up stuff.
>
>> That's one of the reasons I preferred partial function applications to have their own separate syntax and RFC, and have pipes work on just closures/callables directly.  It neatly sidesteps this issue, and leaves $$ free for the "closure that takes an object" syntax.
>> So the above would become something like:
>> $a = $obj->getConfig()
>>  |> array_map($$->getId(), ?)
>>
>> Which is less ambiguous.
>
> What if it's not an object? Let's say an array of arrays? Or an array
> of value objects with public properties, and I don't want to call
> ->getId(), instead just ->id?
>
> ```
> $a = $obj->getConfig()
> |> array_map($$["id"], ?)
> ```

Another example of where "obvious" is not always obvious.

> Personally I would keep the $$ for the pipe. Why:
> 1. Hacklang has it that way.
> 2. I think a lot of people already associate it with this purpose.
> 3. I feel that $$ more has “something previously” or “something same”.
> As it would be passed through the pipe I feel it fits more.
> 4. Maybe the following example is silly but

Hacklang is the *only* language that does it that way.  Every other language with a pipe, or that has considered a pipe, uses a callable on the RHS.  See the notes in the RFC:

https://wiki.php.net/rfc/pipe-operator-v2

Expression on the RHS (using $$ as the insertion operator) is considerably less flexible, and precludes incorporating higher-order functions into the process.  (Like those in https://github.com/Crell/fp/tree/master/src)  Hack simply got this wrong, IMO, and I would likely vote against an RFC with that approach as broken, as it would make it far harder to add the other functional features that are related to pipes. (PFA, a compose operator, etc.)

Which would also free up $$ for some kind of "implied object" syntax, as this thread was originally discussing. :-)

--Larry Garfield

Rowan Collins

3 years ago

On 13 April 2023 04:59:56 BST, Eugene Sidelnyk <zsidelnik@gmail.com> wrote:

>It would be a great pleasure to have the ability to wrap the instance
>method the same way.

The first-class callable syntax works just fine with instance methods, but you need to use it on a particular instance: $someInstance->someMethod(...)

Unfortunately, this is no use for your example, essentially because PHP (unlike, say, Python) doesn't consider $this to be part of the parameters to a method: Verification::getId(...) would mean fn($item) => Verification::getId($item) but you can't call an instance method that way.

What you want is fn($item) => $item->getId(). A syntax like ...->getId() wouldn't be much use - it wouldn't be able to check for the method's existence in advance (the "call to non-static" error in your example happens as soon as you try to create the closure https://3v4l.org/G8va8), or copy over any type information, because there's no way to deduce what class will be used later.

So maybe it would need some other indicator, like Verification::getId(instance ...) to indicate that you want a closure with an extra parameter to be used as the instance. Maybe someone has a less ugly suggestion?

Regards,

--
Rowan Tommins
[IMSoP]