Fixes PHP bug 66331

[mkoppanen]

At the moment userland classes cannot inherit from an extension class that is using ZEND_SEND_PREFER_REF instead of ZEND_SEND_REF.

This bug manifests in for example php-memcached-dev/php-memcached#126

I cannot figure out a good test to write for this issue because it requires an internal class.

[nikic]

I think the current behavior is correct here. A prefer-ref argument cannot be substituted with either a by-val or a by-ref argument while preserving the signature semantics. If it's substituted by a by-val argument, it's no longer possible to pass undefined variables to it. If it's substituted by a by-ref argument, it's no longer possible to pass literals to it. Either way, the signatures are incompatible.

[mkoppanen]

Hello,

it seems that it's impossible to inherit and override a method from an internal class with such a signature without receiving a notice. This is problem especially with testing tools such as PHPUnit, which I believe mock all methods.

Is the only recommended solution to remove PREFER_REF signatures?

[nikic]

@mkoppanen Yes, I think that would be the recommended solution. Alternatively, or rather additionally, it may be worthwhile to start a discussion about allowing to pass a literal null to reference arguments with a null default value. It would make sense that if the argument is optional, you can skip it by passing null. However, that's more of a long-term solution.

[krakjoe]

I think I may have personally provoked this PR, I have vague memories regarding this.

It seems I was wrong, and that we cannot merge this, so I'm closing this PR.

If I'm wrong to do that Nikita will open it again and give us more input ... although it seems open and shut case.

[sodabrew]

I just did a close read through of the original php-memcached-dev/php-memcached#126 and I have two takeaways:

The approach that php-memcached ultimately took, with a flag on the get method that changes the return value to an array, works great with the new PHP 7.1 list() / [] array destructuring left hand assignment. So this ended up being a win:

['value' => $val, 'cas' => $cas, 'flags' => $flags] = $memcached->get('key', null, Memcached::GET_EXTENDED);

The ability to pass null into an argument position that expects a reference but has a default of null, seems sane to me. Discussion is probably out of scope for a GitHub issue? What's the PHP way to bring up a subtle language change like that?