[RFC][Discussion] Consistent callables

  99217
May 28, 2017 21:23 danack@basereality.com (Dan Ackroyd)
Hi,

I'd like to introduce an RFC to cleanup the behaviour of callables and
related functions in PHP.

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

Note, this RFC targets PHP 8, with soft deprecations in PHP 7.3 and
deprecation notices in 7.last. This means there is a lot of time for
reading and thinking about the RFC.

One piece of feedback I've had from informal discussions is that the
BC break for one part is too big even for a major release number. I'd
like to get other people's opinion on the RFC before possibly
modifying it.

Additionally modifying the signature of is_callable() might not be the
best path. Again, looking forward to getting the opinion of other
people.

cheers
Dan
Ack
  99223
May 29, 2017 09:28 php-lists@koalephant.com (Stephen Reay)
> On 29 May 2017, at 04:23, Dan Ackroyd <danack@basereality.com> wrote: > > Hi, > > I'd like to introduce an RFC to cleanup the behaviour of callables and > related functions in PHP. > > https://wiki.php.net/rfc/consistent_callables > > Note, this RFC targets PHP 8, with soft deprecations in PHP 7.3 and > deprecation notices in 7.last. This means there is a lot of time for > reading and thinking about the RFC. > > One piece of feedback I've had from informal discussions is that the > BC break for one part is too big even for a major release number. I'd > like to get other people's opinion on the RFC before possibly > modifying it. > > Additionally modifying the signature of is_callable() might not be the > best path. Again, looking forward to getting the opinion of other > people. > > cheers > Dan > Ack > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
Hi Dan, While there *is* a difference, your resulting comments are reversed for the "call_user_func different from is_callable” section. The call_user_func invocation works properly, the direct invocation does not. A *lot* of the things you’re highlighting as “errors” or “is_callable lied to us” are neither - they’re just a consequence of scope, and I think the wording should be changed, because “it lied” has very different meaning to “it’s response is not necessary applicable in a different scope”. If an astronaut on the moon says he weighs 11.5kg, but you are looking at his medical report on Earth and it says he weighs 69kg, would you call him a liar? Regarding the big change you suggest, making protected/private methods return false unless a new third parameter is true in is_callable(): I think you need to make your intention a *lot* clearer under the "Private and protected functions no longer report as callable for is_callable” section. You make no mention of it being user controlled, simply “this will not work any more”, until the next section. As for the actual change: I think for the sake of BC, and frankly logic, it would make *more* sense, to reverse your flag, so the default respects current behaviour, *but* it gives people who rely on is_callable() from another scope the ability to make sure they only get universally-callable values. Lastly - you mention the need to use Reflection to create closures, but Closure::fromCallable exists, so perhaps you drafted this before that passed, and haven’t updated that part? Cheers, Stephen
  99224
May 29, 2017 09:48 narf@devilix.net (Andrey Andreev)
Hi,

On Mon, May 29, 2017 at 12:28 PM, Stephen Reay <php-lists@koalephant.com> wrote:
> > Regarding the big change you suggest, making protected/private methods return false unless a new third parameter is true in is_callable(): > > I think you need to make your intention a *lot* clearer under the "Private and protected functions no longer report as callable for is_callable” section. You make no mention of it being user controlled, simply “this will not work any more”, until the next section. >
I too noticed this, and the RFC content can be improved in general to be more easily understandable. But that being said, I wholeheartedly agree with the entire proposal. Cheers, Andrey.
  99228
May 29, 2017 11:01 rowan.collins@gmail.com (Rowan Collins)
On 28 May 2017 22:23:30 BST, Dan Ackroyd <danack@basereality.com> wrote:
>Hi, > >I'd like to introduce an RFC to cleanup the behaviour of callables and >related functions in PHP. > >https://wiki.php.net/rfc/consistent_callables
Hi Dan, I like the principle behind this RFC, so thanks for tackling it. A few comments came to mind from a first read through: In your strict definition of a callable, you mention "a string at index 0 which is a valid class name"; this should probably say "a fully qualified class name", i.e. Foo:: class not 'Foo'. This would fit with your proposal to remove support for 'self' and 'static', as the meaning of 'Foo' depends on the current namespace and aliases. Regarding is_callable, I appreciate you want to keep the parameter list short, but I think replacing an existing parameter without any version where it's removed completely could lead to some very confusing bugs. IMO, when BC breaks are necessary, they should be big and obvious so people spot and fix them. Finally, I notice you target a few deprecations at "7.last"; the problem with that is that we might not know when we release that version that it will be the last. It wasn't decided until nearly a year after 5.6 was released that there wouldn't be a 5.7, and the idea of creating a 5.7 containing only deprecations was ruled out. Suggestions to plot a roadmap of future releases have so far but gained traction, so it's likely that 8.0 will be a similar scenario. Regards, -- Rowan Collins [IMSoP]
  100373
September 4, 2017 20:05 danack@basereality.com (Dan Ackroyd)
Stephen Reay wrote:
> Regarding the big change you suggest, making protected/private methods return false unless a new third parameter is true in is_callable(): > I think you need to make your intention a *lot* clearer
On 29 May 2017 at 12:01, Rowan Collins collins@gmail.com> wrote:
> Regarding is_callable, ....could lead to some very confusing bugs. IMO, when BC breaks are necessary, they should be big and obvious so people spot and fix them.
Most of the feedback I got was how modifying the existing is_callable function was dumb, so I've updated the RFC, and dropped the idea of modifying the existing is_callable function. That function will stay as it is, and will be used to determine if a variable can be called in the current scope, through either direct invocation or call_user_func(). The RFC instead now proposes a separate function of is_callable_type, to determine if a variable can be used as callable in all scopes, and so will pass the callable type check for parameters and return types. cheers Dan Ack
  100375
September 5, 2017 03:00 php-lists@koalephant.com (Stephen Reay)
Sent from my iPhone
> On 5 Sep 2017, at 03:05, Dan Ackroyd <danack@basereality.com> wrote: > > Stephen Reay wrote: >> Regarding the big change you suggest, making protected/private methods return false unless a new third parameter is true in is_callable(): >> I think you need to make your intention a *lot* clearer > >> On 29 May 2017 at 12:01, Rowan Collins collins@gmail.com> wrote: >> Regarding is_callable, ....could lead to some very confusing bugs. IMO, when BC breaks are necessary, they should be big and obvious so people spot and fix them. > > > Most of the feedback I got was how modifying the existing is_callable > function was dumb, so I've updated the RFC, and dropped the idea of > modifying the existing is_callable function. > > That function will stay as it is, and will be used to determine if a > variable can be called in the current scope, through either direct > invocation or call_user_func(). > > The RFC instead now proposes a separate function of is_callable_type, > to determine if a variable can be used as callable in all scopes, and > so will pass the callable type check for parameters and return types. > > cheers > Dan > Ack > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
Hi Dan, I appreciate that you've listened to our feedback! As a userland-only dev I'm not really familiar with the trade offs/gotchas inherent to how parameters are handled in the core. Could you/someone explain/identify why a new function is better than converting the `$syntaxonly` bool parameter into a bitmask of CALLABLE_* constants, which treats Boolean true as CALLABLE_SYNTAXONLY for bc purposes? Cheers Stephen Sent from my iPhone