Skip to content

Remove XFAIL from tests for 'bug' 48770 #5386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

alexdowad
Copy link
Contributor

Bug 48770 was opened due to conflicting expectations about the behavior of:

call_user_func_array(array($this, 'parent::methodName'), array($arg1,$arg2))

The one reporting the 'bug' seemed to think that since the method name was prefixed with
parent::, it should call the method in the superclass of the class where this code appears.
However, $this might be an instance of a subclass. If so, then it is quite reasonable that
call_user_func_array will call the method as defined in the superclass of the receiver.

So the 'bug' is not really a bug. Therefore, there is no need for an XFAIL in the tests. They
should just pass.

Amend tests to reflect the actual expected behavior of call_user_func_array, not what the
person who reported bug 48770 thought it should be.

The fact that the PHP interpreter crashes with an error message on the test code is not
really an issue either. The test code is essentially no different from:

function infinite_loop() {
  infinite_loop();
}
infinite_loop();

If anything, perhaps the error message which PHP displays on a stack overflow could be
improved.

Bug 48770 was opened due to conflicting expectations about the behavior of:

    call_user_func_array(array($this, 'parent::methodName'), array($arg1,$arg2))

The one reporting the 'bug' seemed to think that since the method name was prefixed with
`parent::`, it should call the method in the superclass of the *class where this code appears*.
However, `$this` might be an instance of a subclass. If so, then it is quite reasonable that
`call_user_func_array` will call the method as defined in the superclass of *the receiver*.

So the 'bug' is not really a bug. Therefore, there is no need for an XFAIL in the tests. They
should just pass.

Amend tests to reflect the actual expected behavior of `call_user_func_array`, not what the
person who reported bug 48770 thought it should be.

The fact that the PHP interpreter crashes with an error message on the test code is not
really an issue either. The test code is essentially no different from:

    function infinite_loop() {
      infinite_loop();
    }
    infinite_loop();

If anything, perhaps the error message which PHP displays on a stack overflow could be
improved.
@alexdowad
Copy link
Contributor Author

If anything, perhaps the error message which PHP displays on a stack overflow could be
improved.

Oooh... looks like stack space for PHP function calls is actually allocated in the heap using emalloc. It's kind of tricky to catch "stack overflows" if that is the case... unless an arbitrary limit to stack depth was set.

The current error message would definitely confuse a newbie very badly:

Fatal error: Allowed memory size of 134217728 bytes exhausted at /home/alex/Programming/php/php-src/Zend/zend_execute.c:181 (tried to allocate 262176 bytes) in /home/alex/Programming/php/test.php on line 3

It would be really nice to show people a message which would point them in the right direction. But how to do it...?

@cmb69
Copy link
Member

cmb69 commented Apr 14, 2020

@alexdowad, the usual recommendation is to use Xdebug with a suitable max_nesting_level.

@alexdowad
Copy link
Contributor Author

@alexdowad, the usual recommendation is to use Xdebug with a suitable max_nesting_level.

OK. Thanks!

@php-pulls php-pulls closed this in 907bf78 Apr 14, 2020
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