Skip to content

Conversation

dstogov
Copy link
Member

@dstogov dstogov commented Aug 16, 2016

Fixed bug #72598 (Reference is lost after array_slice()).

…ossible.

Fixed bug #72598 (Reference is lost after array_slice()).
@nikic
Copy link
Member

nikic commented Aug 16, 2016

What about call_user_func_array() not going through SEND_ARRAY (e.g. in namespace)? I think this code doesn't handle it yet.

@dstogov
Copy link
Member Author

dstogov commented Aug 16, 2016

Regular call_user_func_array() is changed in the same way.

@nikic
Copy link
Member

nikic commented Aug 16, 2016

Ah, sorry, I missed part of the patch. However, I don't think those implementations do the same thing wrt how the argument is fetched. E.g. call_user_func_array('func', $undefined) will throw a notice for $undefined, while the same in a namespace will not. Similarly for call_user_func_array('func', $array[$key]) in one case this will do a FETCH_DIM_R and in the other (effectively) a FETCH_DIM_W. (Unless I misremember how PREFER_REF handling works, which may well be...)

@dstogov
Copy link
Member Author

dstogov commented Aug 16, 2016

You are right. Probably it's not very hard to fix this (using _W mode for IS_VAR/IS_CV arrays).

What do you think about this solution in general.
Is it better or worse than yours? (I don't have strong opinion).

@bwoebi
Copy link
Member

bwoebi commented Aug 16, 2016

The changes to the tests are quite what I expect. Travis reports a segfault for these tests though...

@nikic Right, but it will throw a type error or at least emit a warning+zpp failure for undefined instead here? [but yeah, should never emit a notice then]

@dstogov
Copy link
Member Author

dstogov commented Aug 16, 2016

All mentioned problems should be fixed now.

@nikic
Copy link
Member

nikic commented Aug 17, 2016

@dstogov About this approach in general, I'm not totally sure. One the one hand, this is a "pragmatic" solution, in that it usually will do what you want. However, this will probably introduce a few gotchas. For example, it is very common that the arguments passed to call_user_func_array() actually come from somewhere else, e.g.

function doCall($args) {
    call_user_func_array($fnWithRefArgs, $args);
}

Currently, if $args is not correctly prepared with references, this will generate a warning. After this change, the call will be allowed, silently, but the passed arguments will stay the same due to the intermediate by-value pass of $args.

@nikic
Copy link
Member

nikic commented Aug 17, 2016

In any case, I don't have a strong opinion either way. I think it may be good to discuss what to do here on the mailing list, as this is a larger issue.

@dstogov
Copy link
Member Author

dstogov commented Aug 18, 2016

@nikic After all, I would prefer your #2059
It breaks few tests, but I hope, it shouldn't be a big problem to fix them.

@dstogov
Copy link
Member Author

dstogov commented Aug 23, 2016

Closed in favor to #2088

@dstogov dstogov closed this Aug 23, 2016
@dstogov dstogov deleted the bug72598 branch October 14, 2019 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants