Hello everyone,
as per the RFC process I'd like to start a discussion on the RFC
I've put up on the RFC Wiki:
https://wiki.php.net/rfc/catchable-call-to-member-of-non-object
RFC: Catchable "call to a member function of a non-object", 1.0
The essence of this pull request is that it converts this fatal error
into E_RECOVERABLE_ERRORs. Framework authors may choose to convert
these into exceptions, as they already do with argument type mismatches.
Feedback is welcome!
- Timm
The example from the Introduction looks like this:
// ...when getAction()returns null:
$this->getAction()->invoke();
My first thought is that if getAction is declared with a return type as per
the currently under discussion return type RFC, then when a null is
returned you'd get an E_RECOVERABLE error there already. I realize there
are other cases and motivations for this RFC, but this is one motivating
case for the return type RFC in the first place.
Hi,
Hello everyone,
as per the RFC process I'd like to start a discussion on the RFC
I've put up on the RFC Wiki:https://wiki.php.net/rfc/catchable-call-to-member-of-non-object
RFC: Catchable "call to a member function of a non-object", 1.0The essence of this pull request is that it converts this fatal error
into E_RECOVERABLE_ERRORs. Framework authors may choose to convert
these into exceptions, as they already do with argument type mismatches.
The definition of E_ERROR
is "an error where the engine can't fully
recover", so if we can recover there should be no need for a vote ;-)
But I have doubts whether this works reliably in all cases where
functions are called. i.e. how will usort()
behave? From a very quick
grep it seems that i.e. sapi/phpdbg doesn't check the return value of
zend_call_function, these things have to be reviewed too. Right now code
(should not but) could rely on the engine doing a bailout which will
shutdown the request on errors to call a function/method.
johannes
Hi Johannes,
The definition of
E_ERROR
is "an error where the engine can't fully
recover", so if we can recover there should be no need for a vote ;-)
:)
But I have doubts whether this works reliably in all cases where
functions are called. i.e. how willusort()
behave?
You're right, there's not test for this. I've added two phpt-files
showing this does not corrupt eval() or calls insideusort()
:
https://github.com/thekid/php-src/commit/da1db5e688c46a044bc0745c86cc34cc4e9ab808
https://github.com/thekid/php-src/commit/7041ef44a6c18bb8d76dc5cf17d20ec00367d24b
From a very quick grep it seems that i.e. sapi/phpdbg doesn't check the
return value of zend_call_function, these things have to be reviewed too.
Good point! Looking into the details of my implementation again though,
this cannot lead to a problem AFAIS, zend_call_function() will return
cleanly in any case. I'm not firm with sapi/phpdbg's usage, could you point
to where you think I'd start seeing a problem?
What I've tried so far is:
$ cat dbg.php
<?php
set_error_handler(function($code, $message) {
var_dump($code, $message);
}, E_RECOVERABLE_ERROR);
$comparator= null;
var_dump($comparator->compare(1, 2));
echo "Alive\n";
$ ./sapi/phpdbg/phpdbg
[Welcome to phpdbg, the interactive PHP debugger, v0.3.2]
To get help using phpdbg type "help" and press enter
[Please report bugs to http://github.com/krakjoe/phpdbg/issues]
phpdbg> exec dbg.php
[Set execution context: /home/friebe/devel/php-src/dbg.php]
phpdbg> compile
[Attempting compilation of /home/friebe/devel/php-src/dbg.php]
[Success]
phpdbg> run
int(4096)
string(51) "Call to a member function compare() on a non-object"
NULL
Alive
phpdbg>
...and also "step 1" and the "next"ing through the code, without any surprises.
- Timm
Hello everyone,
as per the RFC process I'd like to start a discussion on the RFC
I've put up on the RFC Wiki:https://wiki.php.net/rfc/catchable-call-to-member-of-non-object
RFC: Catchable "call to a member function of a non-object", 1.0The essence of this pull request is that it converts this fatal error
into E_RECOVERABLE_ERRORs. Framework authors may choose to convert
these into exceptions, as they already do with argument type mismatches.Feedback is welcome!
Besides the technical issues that Johannes already mentions, I am also
against this for logic reasons.
Calling methods on NULL
objects is a programming mistake. You either
have a bug in the code that creates the objects, and in cases where it
is somethings OK to not have an object, you forgot to check whether you
have an objects. Programming mistakes should not be hidden by error
handlers.
cheers,
Derick
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug
Posted with an email client that doesn't mangle email: alpine
Derick Rethans wrote on 29.04.2014 10:26:
Hello everyone,
as per the RFC process I'd like to start a discussion on the RFC
I've put up on the RFC Wiki:Â https://wiki.php.net/rfc/catchable-call-to-member-of-non-object
 RFC: Catchable "call to a member function of a non-object", 1.0The essence of this pull request is that it converts this fatal error
into E_RECOVERABLE_ERRORs. Framework authors may choose to convert
these into exceptions, as they already do with argument type mismatches.Feedback is welcome!
Besides the technical issues that Johannes already mentions, I am also
against this for logic reasons.Calling methods on
NULL
objects is a programming mistake. You either
have a bug in the code that creates the objects, and in cases where it
is somethings OK to not have an object, you forgot to check whether you
have an objects. Programming mistakes should not be hidden by error
handlers.
Hello Derick,
the RFC is not about hiding programming mistakes, it is still possible to return false in the error handler.
Btw. PHP tolerates a lot of programming mistakes, see Warning, Notice and that's why we like PHP.
In most cases (like concurrent deletes), additional checks take a lot of time for implementation and testing and in most cases, the code is never executed.
There are many functions (like mysqli::query, mysqli::prepare) that return false|null|object. Covering everything with if(...) makes the code bigger and less readable.
Regards,
Thomas
Programming mistakes should not be hidden by error
Of course, but what about having a PHPUnit tests that hard-fail due to a
"PHP Fatal error: Call to a member function foo() on a non-object in ..."?
Just being able to have a handler that catches it as an error is pretty ok,
and it surely allows me to see if there are other failures in a huge test
suite
without having to come up with some crazy code just to run the tests one by
one.
This use case already makes it worth having this improvement in my opinion.
Cheers,
Marco Pivetta
Am 29.04.2014 12:50, schrieb Marco Pivetta:
Of course, but what about having a PHPUnit tests that hard-fail due to a
"PHP Fatal error: Call to a member function foo() on a non-object in ..."?
That is the reason why I am in favor of this; obviously ;-)
Programming mistakes should not be hidden by error
Of course, but what about having a PHPUnit tests that hard-fail due to a
"PHP Fatal error: Call to a member function foo() on a non-object in ..."?Just being able to have a handler that catches it as an error is pretty ok,
and it surely allows me to see if there are other failures in a huge test
suite
without having to come up with some crazy code just to run the tests one by
one.
It's not unthinkable that such a scenario may occur in a production system,
even if the tests passed; when that happens you would now get a WSOD
instead of being able to log/mail the error and show an appropriate error
page.
This use case already makes it worth having this improvement in my opinion.
Cheers,
Marco Pivetta
--
Tjerk