Dear Internals,
The "Improved Error Callback Mechanism" RFC is now in voting phase.
You can cast your vote on the Wiki [1] and the according patch is
available as a Pull Request [2].
Vote will be open for two weeks, counting from today.
Kind regards,
[1] https://wiki.php.net/rfc/improved_error_callback_mechanism
[2] https://github.com/php/php-src/pull/1247
--
Olivier Garcia
Hi all,
Am 28.04.2015 um 15:24 schrieb Olivier Garcia:
The "Improved Error Callback Mechanism" RFC is now in voting phase.
just to satisfy my personal curiosity: how can this RFC target 7.0 which
had the feature freeze over a month ago? Should it not be 7.1 just to
treat all contributors equal?
Greets
Dennis
Hi Dennis,
Le mar. 28 avr. 2015 à 18:33, Dennis Birkholz dennis@birkholz.biz a
écrit :
just to satisfy my personal curiosity: how can this RFC target 7.0 which
had the feature freeze over a month ago? Should it not be 7.1 just to
treat all contributors equal?
We are fully aware of the feature freeze and have no intention to bypass
anything.
This is not about a feature "per se": it's just about being able to hook at
some place in the engine with no userland changes, BC breaks or even ABI
changes. In fact, something that doesn't require an RFC at all (like the
many commits happening in master, not bound to any specific RFC).
Can we consider that this RFC is for documentation purpose mostly? This is
how we see it and the reason we created it.
Cheers,
Patrick
Hi Olivier
2015-04-28 15:24 GMT+02:00 Olivier Garcia oliviergarcia@php.net:
Dear Internals,
The "Improved Error Callback Mechanism" RFC is now in voting phase.
I should probably have been faster at replying, but for PHP7 this is a
no-go. I realize this is a pure internal change and have nothing to do
with userland, but as currently is, we are in feature freeze and it
creates a bad precedence between us as a team if we let this through
now, sorry!
Once we rebranch master to 7.1, I suggest to re-post this RFC.
--
regards,
Kalle Sommer Nielsen
kalle@php.net
I should probably have been faster at replying, but for PHP7 this is a
no-go. I realize this is a pure internal change and have nothing to do
with userland, but as currently is, we are in feature freeze and it
creates a bad precedence between us as a team if we let this through
now, sorry!
Should it not be 7.1 just to treat all contributors equal?
Hi guys,
I definitly don't want any bad blood between PHP community members.
Actually, when I've read the php7timeline RFC and the feature freeze,
I understood it was to limit new userland or BIC features, not for
internal changes, a fortiori when it's mainly about refactoring and
easing extension collaboration.
Cheers,
--
Olivier
Le mar. 28 avr. 2015 à 20:42, Kalle Sommer Nielsen kalle@php.net a écrit :
I should probably have been faster at replying, but for PHP7 this is a
no-go. I realize this is a pure internal change and have nothing to do
with userland, but as currently is, we are in feature freeze and it
creates a bad precedence between us as a team if we let this through
now, sorry!
"No go"? Isn't that a bit rough?
That kind of change normally doesn't require an RFC at all. We did one for
documentation purpose instead of just a PR and a mail saying "ok for 7?".
Refusing this because we actually did an RFC to document it goes in the
opposite direction of encouraging people to create them.
Isn't this just a documented "no brainer"?
Cheers,
Patrick
Le mar. 28 avr. 2015 à 20:42, Kalle Sommer Nielsen kalle@php.net a écrit :
I should probably have been faster at replying, but for PHP7 this is a
no-go. I realize this is a pure internal change and have nothing to do
with userland, but as currently is, we are in feature freeze and it
creates a bad precedence between us as a team if we let this through
now, sorry!"No go"? Isn't that a bit rough?
That kind of change normally doesn't require an RFC at all. We did one for
documentation purpose instead of just a PR and a mail saying "ok for 7?".
I don't agree. This adds nine API functions and a few hundred lines of
code: if you had just committed this, I have no doubt that you'd
have people asking you to revert it pending an RFC and a vote. This
isn't a bug fix.
Just because a feature is internal doesn't mean you can ignore the RFC
process. RFCs aren't just for language features: if they were,
Benjamin and I wouldn't have gone through the hassle of creating an
RFC for making gc_collect_cycles hookable a few months ago.
Refusing this because we actually did an RFC to document it goes in the
opposite direction of encouraging people to create them.Isn't this just a documented "no brainer"?
It's a good feature, the RFC is well written, and I have no doubt this
would be accepted for PHP 7.1. Indeed, it's something I could actually
make good use of in my day job.
None of that is the point.
We — as a development community — agreed to stop adding new features
to PHP 7.0 on a certain date so that we could work on stabilising what
we had and getting it out the door this year. That date was over a
month ago.
So, please, show respect for the people working hard on PHP 7.0 by not
trying to push something in against our agreed processes and making
more work for them.
Adam
Le mar. 28 avr. 2015 à 20:42, Kalle Sommer Nielsen kalle@php.net a
écrit :I should probably have been faster at replying, but for PHP7 this is a
no-go. I realize this is a pure internal change and have nothing to do
with userland, but as currently is, we are in feature freeze and it
creates a bad precedence between us as a team if we let this through
now, sorry!"No go"? Isn't that a bit rough?
That kind of change normally doesn't require an RFC at all. We did one
for
documentation purpose instead of just a PR and a mail saying "ok for
7?".I don't agree. This adds nine API functions and a few hundred lines of
code: if you had just committed this, I have no doubt that you'd
have people asking you to revert it pending an RFC and a vote. This
isn't a bug fix.Just because a feature is internal doesn't mean you can ignore the RFC
process. RFCs aren't just for language features: if they were,
Benjamin and I wouldn't have gone through the hassle of creating an
RFC for making gc_collect_cycles hookable a few months ago.Refusing this because we actually did an RFC to document it goes in
the
opposite direction of encouraging people to create them.Isn't this just a documented "no brainer"?
It's a good feature, the RFC is well written, and I have no doubt this
would be accepted for PHP 7.1. Indeed, it's something I could actually
make good use of in my day job.None of that is the point.
We — as a development community — agreed to stop adding new features
to PHP 7.0 on a certain date so that we could work on stabilising what
we had and getting it out the door this year. That date was over a
month ago.
I see much more impacting change than this one. Yes it adds functions but
not really new codes, I see as a refactoring.
So, please, show respect for the people working hard on PHP 7.0 by not
trying to push something in against our agreed processes and making
more work for them.
I would suggest then to end the white card given with the php7 rfc and
consider to start releases and focus only on stabilization, not perf or
related areas.
Stabilization also includes APIs stabilization as we hopefully get more
people work on getting exts ported.
Cheers,
Pierre
Le mer. 29 avr. 2015 à 03:19, Pierre Joye pierre.php@gmail.com a écrit :
On 28 April 2015 at 15:10, Patrick ALLAERT patrickallaert@php.net
wrote:Le mar. 28 avr. 2015 à 20:42, Kalle Sommer Nielsen kalle@php.net a
écrit :I should probably have been faster at replying, but for PHP7 this is a
no-go. I realize this is a pure internal change and have nothing to do
with userland, but as currently is, we are in feature freeze and it
creates a bad precedence between us as a team if we let this through
now, sorry!"No go"? Isn't that a bit rough?
That kind of change normally doesn't require an RFC at all. We did one
for
documentation purpose instead of just a PR and a mail saying "ok for
7?".I don't agree. This adds nine API functions and a few hundred lines of
code: if you had just committed this, I have no doubt that you'd
have people asking you to revert it pending an RFC and a vote. This
isn't a bug fix.Just because a feature is internal doesn't mean you can ignore the RFC
process. RFCs aren't just for language features: if they were,
Benjamin and I wouldn't have gone through the hassle of creating an
RFC for making gc_collect_cycles hookable a few months ago.Refusing this because we actually did an RFC to document it goes in
the
opposite direction of encouraging people to create them.Isn't this just a documented "no brainer"?
It's a good feature, the RFC is well written, and I have no doubt this
would be accepted for PHP 7.1. Indeed, it's something I could actually
make good use of in my day job.None of that is the point.
We — as a development community — agreed to stop adding new features
to PHP 7.0 on a certain date so that we could work on stabilising what
we had and getting it out the door this year. That date was over a
month ago.I see much more impacting change than this one. Yes it adds functions but
not really new codes, I see as a refactoring.
Pierre is right, what you count as hundreds lines of new code is mostly
moving code from one file to another, function definitions, comments and
creation of macros to make the code more uniform/readable.
So, please, show respect for the people working hard on PHP 7.0 by not
trying to push something in against our agreed processes and making
more work for them.I would suggest then to end the white card given with the php7 rfc and
consider to start releases and focus only on stabilization, not perf or
related areas.Stabilization also includes APIs stabilization as we hopefully get more
people work on getting exts ported.
De : Pierre Joye [mailto:pierre.php@gmail.com]
I would suggest then to end the white card given with the php7 rfc
+1 on this. How can we make it official ? How can we stop the changes merged to the PHP 7 branch every day ? Start a 7.1 branch ?
François
On Wed, Apr 29, 2015 at 12:24 PM, François Laupretre francois@php.net
wrote:
De : Pierre Joye [mailto:pierre.php@gmail.com]
I would suggest then to end the white card given with the php7 rfc
+1 on this. How can we make it official ? How can we stop the changes
merged to the PHP 7 branch every day ? Start a 7.1 branch ?
PHP-7 should be branched out soon, and the RMs should start call out people
when somebody pushes destabilizing changes without prior consensus.
Kalle already asked Dmitry and co about the status of any
pending/in-progress optimizitation/refactoring they are doing, so we can
focus on stabilization and bugfixes.
For 5.6 we tried the following policy: alpha1 means no new RFCs can target
the release, RFCs already under discussion/voting can be finished and
merged before beta1, after tagging beta1 there are no more features coming
in, only fixes.
For PHP7 we had a previously voted on and accepted timeline (
https://wiki.php.net/rfc/php7timeline) which stated marc 15 as the last
time for RFCs targetting php7, and as far as I can tell, everybody seemed
to agree that any RFC targetting PHP7 has to be accepted before that date
(I remember seeing people panic about the scalar type hints voting being
too late and not being able to meet the deadline with the vote).
I just now realized that this RFC was originally put under discussion
before the said deadline, so depending on what others think about it I
would be ok merging it considering that it is still a fairly low risk
change.
Zeev, as the author of the php7timeline, what do you think about merging
this one?
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
De : Ferenc Kovacs [mailto:tyra3l@gmail.com]
For PHP7 we had a previously voted on and accepted timeline (https://wiki.php.net/rfc/php7timeline) which stated marc 15 as
the last time for RFCs targetting php7, and as far as I can tell, everybody seemed to agree that any RFC targetting PHP7
has to be accepted before that date (I remember seeing people panic about the scalar type hints voting being too late
and not being able to meet the deadline with the vote).I just now realized that this RFC was originally put under discussion before the said deadline, so depending on what others think about
it I would be ok merging it considering that it is still a fairly low risk change.
Actually, it was slightly modified and everyone finally agreed that March,15 was the date where the vote should have been started (and not ended, as it was originally intended). This was mainly an artificial way to add a small delay for scalar type hint RFCs.
We never decided that an RFC whose discussion started before that date could go to 7.0. There are a lot in this case (I personally have some) and the consensus is clearly that they have no way to go to 7.0, 'fairly low risk' or not. So, I'm strongly opposed to merging this change in the 7.0 branch.
Regards
François
-----Original Message-----
From: Ferenc Kovacs [mailto:tyra3l@gmail.com]
Sent: Wednesday, April 29, 2015 2:54 PM
To: francois@php.net; Zeev Suraski
Cc: Pierre Joye; Adam Harvey; Patrick ALLAERT; Olivier Garcia; PHP
Internals;
Kalle Sommer Nielsen
Subject: Re: [PHP-DEV] [RFC][VOTE] Improved Error Callback MechanismZeev, as the author of the php7timeline, what do you think about merging
this one?
I think it's way too late to accept anything into 7.0 right now that wasn't
already accepted, even under the most lax of interpretations... We should
do our best to minimize risk to delivering 7.0 on time - and if we
miraculously manage to pull the date in - even better. There'll be life
after 7.0...
Zeev
-----Original Message-----
From: Ferenc Kovacs [mailto:tyra3l@gmail.com]
Sent: Wednesday, April 29, 2015 2:54 PM
To: francois@php.net; Zeev Suraski
Cc: Pierre Joye; Adam Harvey; Patrick ALLAERT; Olivier Garcia; PHP
Internals;
Kalle Sommer Nielsen
Subject: Re: [PHP-DEV] [RFC][VOTE] Improved Error Callback MechanismZeev, as the author of the php7timeline, what do you think about merging
this one?I think it's way too late to accept anything into 7.0 right now that
wasn't
already accepted, even under the most lax of interpretations... We should
do our best to minimize risk to delivering 7.0 on time - and if we
miraculously manage to pull the date in - even better. There'll be life
after 7.0...
I tend to agree and should apply to everything else as well, including perf
improvements. It will definitely help to stabilize and somehow be in time.
Zeev
So, please, show respect for the people working hard on PHP 7.0 [...]
Whoa, the "show respect for the people that work" card already ?
Let's all chill. Here are some kitteh.
http://lol.cat/3z
by not trying to push something in against our agreed processes [...]
We did not push against our processes : you may have not seen it but
we asked for feedbacks on the matter [1] [2], stating from the very
beginning that, to our understanding, it wasn't a feature but code
gardening.
We only got good ones during the discussion phase, hence the change of status.
[1] http://marc.info/?l=php-internals&m=142625551709876&w=2
[2] http://marc.info/?l=php-internals&m=142987114605170&w=2
Hi!
we asked for feedbacks on the matter [1] [2], stating from the very
beginning that, to our understanding, it wasn't a feature but code
gardening.
I think introducing new engine APIs, to be used by extensions, is not
something that should be taken lightly as mere refactoring. If the
externally visible APIs stayed the same, fine, but it's not exactly the
case here.
We only got good ones during the discussion phase, hence the change of status.
I personally missed the original announce in March and had extremely
busy week last week, so I could only take a look at it this week. Yes, I
understand it is frustrating that people take a lot of time to review
things, but APIs are things that tend to stick, so it's best not to rush
them I think.
Stas Malyshev
smalyshev@gmail.com
Hi Patrick
2015-04-29 0:10 GMT+02:00 Patrick ALLAERT patrickallaert@php.net:
"No go"? Isn't that a bit rough?
Well we need to be fair and hold our own policies/processes we agree
to true, else there is no point in having them.
That kind of change normally doesn't require an RFC at all. We did one for
documentation purpose instead of just a PR and a mail saying "ok for 7?".Refusing this because we actually did an RFC to document it goes in the
opposite direction of encouraging people to create them.Isn't this just a documented "no brainer"?
As Adam said, if this was just committed, it would have been asked to
be reverted because now is the time to focus on stabilizing.
If we are to keep our promise by having a release this year, we cannot
afford these small things, and that even involes my tiny improvement
to memory_get[_peak]_usage() the other day as it does not contribute
to that (no matter how redudant my small patch were).
I would love to see more work toward identifying issues in PHP7, and
hopefully we can get that once we release the first alpha.
--
regards,
Kalle Sommer Nielsen
kalle@php.net
Le mer. 29 avr. 2015 à 10:57, Kalle Sommer Nielsen kalle@php.net a écrit :
I would love to see more work toward identifying issues in PHP7, and
hopefully we can get that once we release the first alpha.
The issue targeted here is "two extensions overriding php_error_cb can't
live side by side" and that issue exists since PHP 4.0. Should a bug report
be opened?
We believe that having an extension that additionally hooks in the error
handler mechanism (aside Xdebug) would be of a great help to migrate from
PHP 5.x to 7.0.
Cheers,
Patrick
Hi!
The issue targeted here is "two extensions overriding php_error_cb can't
live side by side" and that issue exists since PHP 4.0. Should a bug report
be opened?
I'm not sure why can't they live side by side. Of course, if the
override is made in incompatible manner, they can't, but the same is
true with proposed API - i.e., one extension can boot all other
extension's hooks with zend_clear_error_hook, potentially breaking all
other extension's functionality.
We believe that having an extension that additionally hooks in the error
handler mechanism (aside Xdebug) would be of a great help to migrate from
PHP 5.x to 7.0.
While I agree with the first part, I'm not sure what makes it vital for
7.0 - as it doesn't introduce BC breaks, is this really important to
have it in major version? Also not sure how it would help migration - if
anything, new API means more work when migrating, not less.
--
Stas Malyshev
smalyshev@gmail.com
Le jeu. 30 avr. 2015 à 01:32, Stanislav Malyshev smalyshev@gmail.com a
écrit :
Hi!
The issue targeted here is "two extensions overriding php_error_cb can't
live side by side" and that issue exists since PHP 4.0. Should a bug
report
be opened?I'm not sure why can't they live side by side.
Since you currently can't hook inside php_error_cb, this is the typical
sequence while loading 2 extensions that want to extend the error callback
mechanism:
// Default error handler
zend_error_cb = php_error_cb;
// Ext 1
zend_error_cb = ext1_error_cb; // Reimplement partially php_error_cb while
adding it's own functionality
// Ext 2
zend_error_cb = ext2_error_cb; // Reimplement partially php_error_cb while
adding it's own functionality
Obviously, ext1_error_cb is never called in this scenario.
Some workarounds have existed, by copying the original error_cb and calling
it additionally, in the above case: ext2_error_cb calling ext1_error_cb
because the original pointer was saved, e.g.: ext1_original_error_cb =
zend_error_cb; right before overriding zend_error_cb. This leads sometimes
to errors being printed twice, because both extensions takes care of
printing the error generated. Xdebug, for example, is not calling the
original error handler for this reason, producing an HTML table with
additional details.
Of course, if the override is made in incompatible manner, they can't,
The current problem is that it is not possible at all to make it in a
compatible way. In addition to that, extensions' error handlers not calling
the original one are forced to copy the original code.
but the same is
true with proposed API - i.e., one extension can boot all other
extension's hooks with zend_clear_error_hook, potentially breaking all
other extension's functionality.
You said it: "potentially" which is a much better situation that "forcibly"
:)
zend_clear_error_hook is there for completeness. If used, it means you know
what you are doing and you forcibly want any previously defined hooks to be
removed.
One use case being: I don't want any error displaying, let's push mine (or
let's have none)!
A more complete API would be to propose a function to selectively remove
one specific hook, this can still be done using zend_llist_del_element on
PG(error_hooks)[E_HOOK_xxx] but we believe we can live without it for now.
We believe that having an extension that additionally hooks in the error
handler mechanism (aside Xdebug) would be of a great help to migrate from
PHP 5.x to 7.0.While I agree with the first part, I'm not sure what makes it vital for
7.0 - as it doesn't introduce BC breaks, is this really important to
have it in major version? Also not sure how it would help migration - if
anything, new API means more work when migrating, not less.
We are talking about PHP users migrating to 7.0, not about PHP extension
migration.
Most PHP developers typically have Xdebug installed and activated, this
prevent the use of extensions that collects errors and provide added value
(like PECL/APM or commercial tools) to the typical display/logging
mechanism of PHP.
Note that it doesn't interfere at all with the job of making all PHP
extensions compatible with 7. Extensions can still override zend_error_cb
like they always did. But at least we can blame the maintainer of not
benefiting from the new API and playing nicely with others ;)
Cheers,
Patrick
Hi!
You can cast your vote on the Wiki [1] and the according patch is
available as a Pull Request [2].Vote will be open for two weeks, counting from today.
I think the idea of this RFC is nice but it needs a bit more work to
make it really good and successful. See some of the notes below and more
technical ones in the patch. I think it would be good not to rush it but
have it for 7.1 with more time to develop it to maturity.
Reading the RFC, I'm not sure I fully understand what is the difference
between E_HOOK_LOG, E_HOOK_DISPLAY and E_HOOK_PROCESS. The code just
calls all of them in a sequence. What is the difference between them and
when I use each of them? Especially not clear about E_HOOK_PROCESS - why
I need it if I could just hook one of the other ones and do whatever I
need to process the error?
Also, there seems to be no link between E_HOOK_LOG and PG(log_errors)
and E_HOOK_DISPLAY and PG(display_errors), so these will be called even
if log/display globals set to disabled. So the code that implements
something like custom error display would have to copy checks from PHP
core code if they want the custom error not appear when display_errors
is off (which they may want to I assume).
If you override bailout callback and request bailout, then the bailout
would happen, but nothing in zend_error_bailout_cb() would - i.e. no
error code setting, no HTTP response code, etc.
Also, zend_error_bailout_cb() actually calls zend_bailout(), so it is
not clear - should the bailout hook call zend_bailout() or return
FAILURE? The comment in the code of php_error_cb() says "hook_result ==
FAILURE means we must bail out" but the bailout does not actually happen
there. Moreover, it is not clear when bailout hook is supposed to return
FAILURE at all.
Also, I see a lot of php_get_module_initialized() checks inside the
specific handlers - but if an extension overrides it, these checks are
not performed, so the extension may still work with uninitialized data,
which would be not good. Of course, extension writer can copy-paste all
these checks but it's easy to forget and if they ever change it would
also be easy to forget to update it.
Stas Malyshev
smalyshev@gmail.com
Le jeu. 30 avr. 2015 à 02:26, Stanislav Malyshev smalyshev@gmail.com a
écrit :
Hi!
You can cast your vote on the Wiki [1] and the according patch is
available as a Pull Request [2].Vote will be open for two weeks, counting from today.
I think the idea of this RFC is nice but it needs a bit more work to
make it really good and successful. See some of the notes below and more
technical ones in the patch. I think it would be good not to rush it but
have it for 7.1 with more time to develop it to maturity.
Thank you for your remarks on the PR, I will handle it ASAP.
Reading the RFC, I'm not sure I fully understand what is the difference
between E_HOOK_LOG, E_HOOK_DISPLAY and E_HOOK_PROCESS. The code just
calls all of them in a sequence. What is the difference between them and
when I use each of them? Especially not clear about E_HOOK_PROCESS - why
I need it if I could just hook one of the other ones and do whatever I
need to process the error?
E_HOOK_LOG:
Logging (typically using zend_append_error_hook)
E_HOOK_DISPLAY:
Displaying the error (typically using zend_prepend_error_hook + potentially
returning "FAILURE" so that it's the only hook treating the "display" part.
Or zend_clear_error_hook + zend_append_error_hook)
E_HOOK_PROCESS:
For stuff not fitting in the log / display category.
We can perfectly live without it, this was for completeness as well.
Better waiting for someone asking for it in order to avoid YAGNI?
E_HOOK_BAILOUT:
Responsible for bailing out.
Also, there seems to be no link between E_HOOK_LOG and PG(log_errors)
and E_HOOK_DISPLAY and PG(display_errors), so these will be called even
if log/display globals set to disabled. So the code that implements
something like custom error display would have to copy checks from PHP
core code if they want the custom error not appear when display_errors
is off (which they may want to I assume).
That is a very valid remark! The design choice we made, which is missing in
the RFC, is that we considered those flags as the one controlling the
"default behaviour" (read: specific to zend_error_display_cb /
zend_error_log_cb) and not the "general" one. We preferred favoring a
generic design which enables hook authors to discard those flags for
whatever good or bad reason at the price of duplicating some logic. For
example, Xdebug provides xdebug.force_display_errors in order to force the
display indepedently of PG(display_errors) [1]
Avoiding a general check around E_HOOK_LOG / E_HOOK_DISPLAY, we hope to
decrease the chance of having a "log" or "display" hook implemented in
E_HOOK_PROCESS for the reason that one want to ignore one of those flags.
We therefore have a preference for keeping the current design, but don't
mind changing if someone brings additional input on the subject. It's a
flexibility vs duplication choice.
If you override bailout callback and request bailout, then the bailout
would happen, but nothing in zend_error_bailout_cb() would - i.e. no
error code setting, no HTTP response code, etc.Also, zend_error_bailout_cb() actually calls zend_bailout(), so it is
not clear - should the bailout hook call zend_bailout() or return
FAILURE? The comment in the code of php_error_cb() says "hook_result ==
FAILURE means we must bail out" but the bailout does not actually happen
there. Moreover, it is not clear when bailout hook is supposed to return
FAILURE at all.
Bailout hooks are responsible for calling zend_bailout() (or implementing a
similar behaviour) AND return FAILURE in order to prevent more hooks of
that category to be run.
zend_bailout() could possibly be called if FAILURE is returned by a bailout
hook instead of by the hook itself, but then there is no way one can
implement zend_bailout differently.
We have no use case for that, so we can perfectly take that direction if
you think it's preferable.
Also, I see a lot of php_get_module_initialized() checks inside the
specific handlers - but if an extension overrides it, these checks are
not performed, so the extension may still work with uninitialized data,
which would be not good. Of course, extension writer can copy-paste all
these checks but it's easy to forget and if they ever change it would
also be easy to forget to update it.
Yes, those calls to php_get_module_initialized() replace the direct access
to module_initialized variable that were performed from within the original
php_error_cb.
Do you have something to propose here, like moving some part of the default
hooks directly into php_error_cb?
Regarding the copy-paste issue, which I fully agree with, note that the
current situation is worst than with the patch because if one want to
change, e.g., the display, it's the whole php_error_cb content (~240 lines)
that has to be copied (or to take inspiration on) vs. the corresponding
hook only (~50 lines for the most complex one: display) while letting all
the other parts intact.
Looking at xdebug_error_cb [2], there's a lot of copy-paste, including
usage of php_get_module_initialized() that can't be avoided for now.
Cheers,
Patrick
[1]
https://github.com/xdebug/xdebug/blob/fcbefa6ca55757d1aaec72511b71bbfc113e33dc/xdebug_stack.c#L599
[2]
https://github.com/xdebug/xdebug/blob/fcbefa6ca55757d1aaec72511b71bbfc113e33dc/xdebug_stack.c#L531
Dear Internals,
The "Improved Error Callback Mechanism" RFC is now in voting phase.
You can cast your vote on the Wiki [1] and the according patch is
available as a Pull Request [2].Vote will be open for two weeks, counting from today.
It's now 6 weeks later and the vote is still open, what is the actual
status of this RFC and its vote?
Kind regards,
[1] https://wiki.php.net/rfc/improved_error_callback_mechanism
[2] https://github.com/php/php-src/pull/1247--
Olivier Garcia
Le mer. 10 juin 2015 à 17:20, Peter Cowburn petercowburn@gmail.com a
écrit :
Dear Internals,
The "Improved Error Callback Mechanism" RFC is now in voting phase.
You can cast your vote on the Wiki [1] and the according patch is
available as a Pull Request [2].Vote will be open for two weeks, counting from today.
It's now 6 weeks later and the vote is still open, what is the actual
status of this RFC and its vote?
Good catch!
I just closed it.
We will reopen it while targeting PHP 7.1. The negative feedback we have
received so far were about the fact it targets PHP 7.0 while in feature
freeze.
Patrick