-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Debug] Remove $context arg from handleError(), preparing for PHP 7.2 #21389
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
Conversation
@@ -353,16 +353,14 @@ private function reRegister($prev) | |||
* @param string $message | |||
* @param string $file | |||
* @param int $line | |||
* @param array $context | |||
* @param array $backtrace | |||
* | |||
* @return bool Returns false when no handling happens so that the PHP engine can handle the error itself | |||
* | |||
* @throws \ErrorException When $this->thrownErrors requests so | |||
* | |||
* @internal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fortunately, this annotation is here from the beginning so we can change the signature without breaking our BC policy.
646b7b9
to
f3211db
Compare
is there still a case where we will provide a context to the exception ? If no, we should deprecate ContextErrorException (which is here only to add the context) |
Done in #21388 |
if (isset($context['GLOBALS']) && ($this->scopedErrors & $type)) { | ||
if (4 < $numArgs = func_num_args()) { | ||
$context = $scope ? func_get_arg(4) : array(); | ||
$backtrace = 5 < $numArgs ? func_get_arg(5) : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is the backtrace comingfrom ? AFAIK, PHP never passes such argument
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HHVM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, can you add a small comment? That way, we can remove this piece of code more easily if we decide to drop HHVM support one day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment added
f3211db
to
83cad14
Compare
Thank you @nicolas-grekas. |
…g for PHP 7.2 (nicolas-grekas) This PR was merged into the 2.7 branch. Discussion ---------- [Debug] Remove $context arg from handleError(), preparing for PHP 7.2 | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - PHP 7.2 will trigger deprecation notices. See https://wiki.php.net/rfc/deprecations_php_7_2#errcontext_argument_of_error_handler Commits ------- 83cad14 [Debug] Remove $context arg from handleError(), preparing for PHP 7.2
PHP 7.2 will trigger deprecation notices.
See https://wiki.php.net/rfc/deprecations_php_7_2#errcontext_argument_of_error_handler