Re: [PHP-DEV] [RFC] Throwable Interface
On Tue, Jun 16, 2015 at 1:41 AM, Marcio Almada marcio.w...@gmail.com wrote: Hi, 2015-06-15 19:25 GMT-03:00 Aaron Piotrowski aa...@icicle.io: On Jun 15, 2015, at 4:02 PM, Anatol Belski anatol@belski.net wrote: I would then suggest Aaron to stick to the minimal voting period (announcing this as early as possible), if the voting passes - then merge the branch on Wednesday. This way we would have nearly one week to test and do fixes in master. Kalle, Ferenc, how do you feel about this? Is changing the voting dates on the RFC to run only through tomorrow allowed? If so, I can make that change so it can be merged this week. Regards, Aaron Piotrowski I second that. Right now many projects are awaiting to move towards php7 testing due to issues like https://github.com/sebastianbergmann/phpunit/issues/1748. It would be great to get the next alpha with the fixed exception hierarchy, in case the release managers agree. Márcio -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php I would be okay with shortening the voting period, given the extensive discussion, having Dmitry and Nikita reviewing the changes and seeing how strong the support for this rfc is(a reasonable amount of votes with unanimously in favor of the change). I think that even if something comes up later with the implementation, we can fix it on the way before the RC phase starts. -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] [RFC] Throwable Interface
Hi, I made a quick code review, and I don't see any technical problems in implementation. 1) Anyway, I think it's a bad idea to rename EngineException (and others) into Error(s). This will prevent using class Error in applications, and may potentially break some of them. I also don't like renaming in ~440 tests (I didn't review all these changes). 2) I think it's better to move zend_ce_throwable definition/initialization from zend_interfaces.h/c into zend_exception.h/c. At least, this will allow arg_info reuse. (it's missed now, but should be added now or in the future). 3) In ext/simplexml/tests/SimpleXMLElement_xpath_3.phpt EngineExcpetion is not renamed into Error. Thanks. Dmitry. On Sun, Jun 14, 2015 at 8:24 PM, Aaron Piotrowski aa...@icicle.io wrote: On Jun 14, 2015, at 11:31 AM, Anatol Belski anatol@belski.net wrote: Hi Dmitry, I would go by accepting this. Furthermore – if you feel that the implementation is stable enough and does not BC, I would suggest to have it already in the alpha2. As there seems to be at all no resistance in the votes (no even single “no” voter yet), nor strong objection here on the lists. The minimal voting period is 1 week, so theoretically if it were ended on Wed (the voting RFC doesn’t disallow this) – there were still some time to do extensive testing and fixes. Alpha2 is the time where a) a lot of users will be able to test it and b) it still can be reverted in the worst case. What do you think? Regards Anatol I will have some time to resolve the merge conflicts later today, so I will be happy to take care of that. Regards, Aaron Piotrowski -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] [RFC] Throwable Interface
Hi Dmitry, -Original Message- From: Dmitry Stogov [mailto:dmi...@zend.com] Sent: Monday, June 15, 2015 10:53 AM To: Aaron Piotrowski Cc: Anatol Belski; PHP Internals Subject: Re: [PHP-DEV] [RFC] Throwable Interface Hi, I made a quick code review, and I don't see any technical problems in implementation. 1) Anyway, I think it's a bad idea to rename EngineException (and others) into Error(s). This will prevent using class Error in applications, and may potentially break some of them. I also don't like renaming in ~440 tests (I didn't review all these changes). 2) I think it's better to move zend_ce_throwable definition/initialization from zend_interfaces.h/c into zend_exception.h/c. At least, this will allow arg_info reuse. (it's missed now, but should be added now or in the future). 3) In ext/simplexml/tests/SimpleXMLElement_xpath_3.phpt EngineExcpetion is not renamed into Error. Thanks for the review. I've also tested the branch which has today's master merged in, by now it doesn't show any functional regression. Actually I part your first concern about Error. Spent some time phishing for class Error on github and found three apps doing this without namespace. But that's from 100 search result pages. And those three apps are forked zero to 1 times, so pretty much low usage. This will likely break more in the world, we hardly know. The current RFC can't be changed while in voting. But how it looks like, the principle of the Trowable RFC is something everyone agrees on. Maybe do another RFC in parallel for better names? Still I'd stand for taking it into alpha2 - if we want to have it, better to arrange it in a way that it doesn't cause unnecessary delays in the release process. Regards Anatol -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
On Jun 15, 2015, at 6:56 AM, Anatol Belski anatol@belski.net wrote:
Hi Dmitry,
-Original Message-
From: Dmitry Stogov [mailto:dmi...@zend.com]
Sent: Monday, June 15, 2015 10:53 AM
To: Aaron Piotrowski
Cc: Anatol Belski; PHP Internals
Subject: Re: [PHP-DEV] [RFC] Throwable Interface
Hi,
I made a quick code review, and I don't see any technical problems in
implementation.
1) Anyway, I think it's a bad idea to rename EngineException (and others)
into
Error(s).
This will prevent using class Error in applications, and may potentially
break
some of them.
I also don't like renaming in ~440 tests (I didn't review all these changes).
2) I think it's better to move zend_ce_throwable
definition/initialization from zend_interfaces.h/c into zend_exception.h/c.
At least, this will allow arg_info reuse. (it's missed now, but should be
added
now or in the future).
3) In ext/simplexml/tests/SimpleXMLElement_xpath_3.phpt EngineExcpetion is
not renamed into Error.
Thanks for the review. I've also tested the branch which has today's master
merged in, by now it doesn't show any functional regression.
Actually I part your first concern about Error. Spent some time phishing for
class Error on github and found three apps doing this without namespace.
But that's from 100 search result pages. And those three apps are forked zero
to 1 times, so pretty much low usage. This will likely break more in the
world, we hardly know.
The current RFC can't be changed while in voting. But how it looks like, the
principle of the Trowable RFC is something everyone agrees on. Maybe do
another RFC in parallel for better names? Still I'd stand for taking it into
alpha2 - if we want to have it, better to arrange it in a way that it doesn't
cause unnecessary delays in the release process.
Hi Dmitry and Anatol,
I fixed three more tests that were missed when merging, as I only checked those
that failed after the merge. I should have used find and replace again after the
merge, which is how I changed the majority of the tests. Only a few tests
required
manual changes, mostly because of hard-coded string lengths, and one or two
tests used the class name Error. Note that Nikita recently changed nearly all
these tests and many others when updating the phrasing on uncaught exception
messages. While many tests were changed, users won’t have such issues
because they aren’t catching these exceptions yet.
Would you like me to move zend_ce_throwable to zend_exceptions.h/c or is that
something you’d take care of after the merge?
I am strongly against naming something _Exception if it doesn’t extend
Exception. This was most of the point of the RFC, as I find code such as
`catch (Exception $e) { … } catch (EngineException) { … }` very unintuitive and
I
feel it will lead to confusion for users.
I sifted through search results on GitHub looking for usage of Error before
proposing the RFC and also found only a couple of actual uses from generally
unused projects. Most of the other results are forks of an old version of
PHPUnit.
Namespaces have been around long enough that most libraries and apps will not
have such a class in the root namespace. As far as BC breaks go, renaming a
class in an app is a fairly trivial one to make, and one that very, very few
people
will have to do. I feel having a simple name such as Error is more important
than
avoiding naming conflicts with a very small amount of code.
Regards,
Aaron Piotrowski
Re: [PHP-DEV] [RFC] Throwable Interface
On Mon, Jun 15, 2015 at 4:55 PM, Aaron Piotrowski aa...@icicle.io wrote:
On Jun 15, 2015, at 6:56 AM, Anatol Belski anatol@belski.net wrote:
Hi Dmitry,
-Original Message-
From: Dmitry Stogov [mailto:dmi...@zend.com dmi...@zend.com]
Sent: Monday, June 15, 2015 10:53 AM
To: Aaron Piotrowski
Cc: Anatol Belski; PHP Internals
Subject: Re: [PHP-DEV] [RFC] Throwable Interface
Hi,
I made a quick code review, and I don't see any technical problems in
implementation.
1) Anyway, I think it's a bad idea to rename EngineException (and
others) into
Error(s).
This will prevent using class Error in applications, and may potentially
break
some of them.
I also don't like renaming in ~440 tests (I didn't review all these
changes).
2) I think it's better to move zend_ce_throwable
definition/initialization from zend_interfaces.h/c into zend_exception.h/c.
At least, this will allow arg_info reuse. (it's missed now, but should be
added
now or in the future).
3) In ext/simplexml/tests/SimpleXMLElement_xpath_3.phpt EngineExcpetion is
not renamed into Error.
Thanks for the review. I've also tested the branch which has today's
master merged in, by now it doesn't show any functional regression.
Actually I part your first concern about Error. Spent some time phishing
for class Error on github and found three apps doing this without
namespace. But that's from 100 search result pages. And those three apps
are forked zero to 1 times, so pretty much low usage. This will likely
break more in the world, we hardly know.
The current RFC can't be changed while in voting. But how it looks like,
the principle of the Trowable RFC is something everyone agrees on. Maybe do
another RFC in parallel for better names? Still I'd stand for taking it
into alpha2 - if we want to have it, better to arrange it in a way that it
doesn't cause unnecessary delays in the release process.
Hi Dmitry and Anatol,
I fixed three more tests that were missed when merging, as I only checked
those
that failed after the merge. I should have used find and replace again
after the
merge, which is how I changed the majority of the tests. Only a few tests
required
manual changes, mostly because of hard-coded string lengths, and one or two
tests used the class name Error. Note that Nikita recently changed nearly
all
these tests and many others when updating the phrasing on uncaught
exception
messages. While many tests were changed, users won’t have such issues
because they aren’t catching these exceptions yet.
Would you like me to move zend_ce_throwable to zend_exceptions.h/c or is
that
something you’d take care of after the merge?
I don't care about this a lot. I just think it's better.
If you don't see any problems, please, move the code.
I am strongly against naming something _Exception if it doesn’t extend
Exception. This was most of the point of the RFC, as I find code such as
`catch (Exception $e) { … } catch (EngineException) { … }` very
unintuitive and I
feel it will lead to confusion for users.
I sifted through search results on GitHub looking for usage of Error before
proposing the RFC and also found only a couple of actual uses from
generally
unused projects. Most of the other results are forks of an old version of
PHPUnit.
Namespaces have been around long enough that most libraries and apps will
not
have such a class in the root namespace. As far as BC breaks go, renaming a
class in an app is a fairly trivial one to make, and one that very, very
few people
will have to do. I feel having a simple name such as Error is more
important than
avoiding naming conflicts with a very small amount of code.
Nikita, what is your opinion?
If you don't care, I won't as well.
Thanks. Dmitry.
Regards,
Aaron Piotrowski
Re: [PHP-DEV] [RFC] Throwable Interface
On Mon, Jun 15, 2015 at 4:53 PM, Dmitry Stogov dmi...@zend.com wrote:
On Mon, Jun 15, 2015 at 4:55 PM, Aaron Piotrowski aa...@icicle.io wrote:
On Jun 15, 2015, at 6:56 AM, Anatol Belski anatol@belski.net wrote:
Hi Dmitry,
-Original Message-
From: Dmitry Stogov [mailto:dmi...@zend.com dmi...@zend.com]
Sent: Monday, June 15, 2015 10:53 AM
To: Aaron Piotrowski
Cc: Anatol Belski; PHP Internals
Subject: Re: [PHP-DEV] [RFC] Throwable Interface
Hi,
I made a quick code review, and I don't see any technical problems in
implementation.
1) Anyway, I think it's a bad idea to rename EngineException (and
others) into
Error(s).
This will prevent using class Error in applications, and may
potentially break
some of them.
I also don't like renaming in ~440 tests (I didn't review all these
changes).
2) I think it's better to move zend_ce_throwable
definition/initialization from zend_interfaces.h/c into
zend_exception.h/c.
At least, this will allow arg_info reuse. (it's missed now, but should be
added
now or in the future).
3) In ext/simplexml/tests/SimpleXMLElement_xpath_3.phpt EngineExcpetion is
not renamed into Error.
Thanks for the review. I've also tested the branch which has today's
master merged in, by now it doesn't show any functional regression.
Actually I part your first concern about Error. Spent some time phishing
for class Error on github and found three apps doing this without
namespace. But that's from 100 search result pages. And those three apps
are forked zero to 1 times, so pretty much low usage. This will likely
break more in the world, we hardly know.
The current RFC can't be changed while in voting. But how it looks like,
the principle of the Trowable RFC is something everyone agrees on. Maybe do
another RFC in parallel for better names? Still I'd stand for taking it
into alpha2 - if we want to have it, better to arrange it in a way that it
doesn't cause unnecessary delays in the release process.
Hi Dmitry and Anatol,
I fixed three more tests that were missed when merging, as I only checked
those
that failed after the merge. I should have used find and replace again
after the
merge, which is how I changed the majority of the tests. Only a few tests
required
manual changes, mostly because of hard-coded string lengths, and one or
two
tests used the class name Error. Note that Nikita recently changed nearly
all
these tests and many others when updating the phrasing on uncaught
exception
messages. While many tests were changed, users won’t have such issues
because they aren’t catching these exceptions yet.
Would you like me to move zend_ce_throwable to zend_exceptions.h/c or is
that
something you’d take care of after the merge?
I don't care about this a lot. I just think it's better.
If you don't see any problems, please, move the code.
I am strongly against naming something _Exception if it doesn’t extend
Exception. This was most of the point of the RFC, as I find code such as
`catch (Exception $e) { … } catch (EngineException) { … }` very
unintuitive and I
feel it will lead to confusion for users.
I sifted through search results on GitHub looking for usage of Error
before
proposing the RFC and also found only a couple of actual uses from
generally
unused projects. Most of the other results are forks of an old version of
PHPUnit.
Namespaces have been around long enough that most libraries and apps will
not
have such a class in the root namespace. As far as BC breaks go, renaming
a
class in an app is a fairly trivial one to make, and one that very, very
few people
will have to do. I feel having a simple name such as Error is more
important than
avoiding naming conflicts with a very small amount of code.
Nikita, what is your opinion?
If you don't care, I won't as well.
I'm fine with these changes. Patch looks okay to me as well.
Nikita
Re: [PHP-DEV] [RFC] Throwable Interface
I think REGISTER_INTERFACE() macro in zend_interfaces.h is better. Actually, it's not related to ITERATORs at all. Thanks. Dmitry. On Tue, Jun 16, 2015 at 12:48 AM, Aaron Piotrowski aa...@icicle.io wrote: On Jun 15, 2015, at 9:53 AM, Dmitry Stogov dmi...@zend.com wrote: On Mon, Jun 15, 2015 at 4:55 PM, Aaron Piotrowski aa...@icicle.io wrote: On Jun 15, 2015, at 6:56 AM, Anatol Belski anatol@belski.net wrote: Would you like me to move zend_ce_throwable to zend_exceptions.h/c or is that something you’d take care of after the merge? I don't care about this a lot. I just think it's better. If you don’t see any problems, please, move the code. Moving the definition of zend_ce_throwable to zend_exceptions.h/c requires either: 1. Moving #define REGISTER_ITERATOR_INTERFACE from zend_interfaces.c to zend_interfaces.h so it is available in zend_exceptions.c (I could take the opportunity to rename this macro to REGISTER_INTERFACE). 2. Defining a macro in zend_exceptions.c that is identical to REGISTER_ITERATOR_INTERFACE. Which option sounds better? Aaron Piotrowski
Re: [PHP-DEV] [RFC] Throwable Interface
On Jun 15, 2015, at 9:53 AM, Dmitry Stogov dmi...@zend.com wrote: On Mon, Jun 15, 2015 at 4:55 PM, Aaron Piotrowski aa...@icicle.io mailto:aa...@icicle.io wrote: On Jun 15, 2015, at 6:56 AM, Anatol Belski anatol@belski.net mailto:anatol@belski.net wrote: Would you like me to move zend_ce_throwable to zend_exceptions.h/c or is that something you’d take care of after the merge? I don't care about this a lot. I just think it's better. If you don’t see any problems, please, move the code. Moving the definition of zend_ce_throwable to zend_exceptions.h/c requires either: 1. Moving #define REGISTER_ITERATOR_INTERFACE from zend_interfaces.c to zend_interfaces.h so it is available in zend_exceptions.c (I could take the opportunity to rename this macro to REGISTER_INTERFACE). 2. Defining a macro in zend_exceptions.c that is identical to REGISTER_ITERATOR_INTERFACE. Which option sounds better? Aaron Piotrowski
RE: [PHP-DEV] [RFC] Throwable Interface
-Original Message-
From: Nikita Popov [mailto:nikita@gmail.com]
Sent: Monday, June 15, 2015 10:36 PM
To: Dmitry Stogov
Cc: Aaron Piotrowski; Anatol Belski; PHP Internals
Subject: Re: [PHP-DEV] [RFC] Throwable Interface
On Mon, Jun 15, 2015 at 4:53 PM, Dmitry Stogov dmi...@zend.com wrote:
On Mon, Jun 15, 2015 at 4:55 PM, Aaron Piotrowski aa...@icicle.io wrote:
On Jun 15, 2015, at 6:56 AM, Anatol Belski anatol@belski.net wrote:
Hi Dmitry,
-Original Message-
From: Dmitry Stogov [mailto:dmi...@zend.com dmi...@zend.com]
Sent: Monday, June 15, 2015 10:53 AM
To: Aaron Piotrowski
Cc: Anatol Belski; PHP Internals
Subject: Re: [PHP-DEV] [RFC] Throwable Interface
Hi,
I made a quick code review, and I don't see any technical problems in
implementation.
1) Anyway, I think it's a bad idea to rename EngineException (and
others) into
Error(s).
This will prevent using class Error in applications, and may
potentially break some of them.
I also don't like renaming in ~440 tests (I didn't review all these
changes).
2) I think it's better to move zend_ce_throwable
definition/initialization from zend_interfaces.h/c into
zend_exception.h/c.
At least, this will allow arg_info reuse. (it's missed now, but
should be added now or in the future).
3) In ext/simplexml/tests/SimpleXMLElement_xpath_3.phpt
EngineExcpetion is not renamed into Error.
Thanks for the review. I've also tested the branch which has today's
master merged in, by now it doesn't show any functional regression.
Actually I part your first concern about Error. Spent some time
phishing for class Error on github and found three apps doing this
without namespace. But that's from 100 search result pages. And those
three apps are forked zero to 1 times, so pretty much low usage. This
will likely break more in the world, we hardly know.
The current RFC can't be changed while in voting. But how it looks
like, the principle of the Trowable RFC is something everyone agrees
on. Maybe do another RFC in parallel for better names? Still I'd
stand for taking it into alpha2 - if we want to have it, better to
arrange it in a way that it doesn't cause unnecessary delays in the release
process.
Hi Dmitry and Anatol,
I fixed three more tests that were missed when merging, as I only
checked those that failed after the merge. I should have used find
and replace again after the merge, which is how I changed the
majority of the tests. Only a few tests required manual changes,
mostly because of hard-coded string lengths, and one or two tests
used the class name Error. Note that Nikita recently changed nearly
all these tests and many others when updating the phrasing on
uncaught exception messages. While many tests were changed, users
won’t have such issues because they aren’t catching these exceptions
yet.
Would you like me to move zend_ce_throwable to zend_exceptions.h/c or
is that something you’d take care of after the merge?
I don't care about this a lot. I just think it's better.
If you don't see any problems, please, move the code.
I am strongly against naming something _Exception if it doesn’t extend
Exception. This was most of the point of the RFC, as I find code such as
`catch (Exception $e) { … } catch (EngineException) { … }` very
unintuitive and I
feel it will lead to confusion for users.
I sifted through search results on GitHub looking for usage of Error
before
proposing the RFC and also found only a couple of actual uses from
generally
unused projects. Most of the other results are forks of an old version of
PHPUnit.
Namespaces have been around long enough that most libraries and apps will
not
have such a class in the root namespace. As far as BC breaks go, renaming
a
class in an app is a fairly trivial one to make, and one that very, very
few people
will have to do. I feel having a simple name such as Error is more
important than
avoiding naming conflicts with a very small amount of code.
Nikita, what is your opinion?
If you don't care, I won't as well.
I'm fine with these changes. Patch looks okay to me as well.
I would then suggest Aaron to stick to the minimal voting period (announcing
this as early as possible), if the voting passes - then merge the branch on
Wednesday. This way we would have nearly one week to test and do fixes in
master.
Kalle, Ferenc, how do you feel about this?
Regards
Anatol
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
Hi, 2015-06-15 19:25 GMT-03:00 Aaron Piotrowski aa...@icicle.io: On Jun 15, 2015, at 4:02 PM, Anatol Belski anatol@belski.net wrote: I would then suggest Aaron to stick to the minimal voting period (announcing this as early as possible), if the voting passes - then merge the branch on Wednesday. This way we would have nearly one week to test and do fixes in master. Kalle, Ferenc, how do you feel about this? Is changing the voting dates on the RFC to run only through tomorrow allowed? If so, I can make that change so it can be merged this week. Regards, Aaron Piotrowski I second that. Right now many projects are awaiting to move towards php7 testing due to issues like https://github.com/sebastianbergmann/phpunit/issues/1748. It would be great to get the next alpha with the fixed exception hierarchy, in case the release managers agree. Márcio -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
On Jun 15, 2015, at 4:02 PM, Anatol Belski anatol@belski.net wrote: I would then suggest Aaron to stick to the minimal voting period (announcing this as early as possible), if the voting passes - then merge the branch on Wednesday. This way we would have nearly one week to test and do fixes in master. Kalle, Ferenc, how do you feel about this? Is changing the voting dates on the RFC to run only through tomorrow allowed? If so, I can make that change so it can be merged this week. Regards, Aaron Piotrowski -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
RE: [PHP-DEV] [RFC] Throwable Interface
Hi Dmitry, I would go by accepting this. Furthermore – if you feel that the implementation is stable enough and does not BC, I would suggest to have it already in the alpha2. As there seems to be at all no resistance in the votes (no even single “no” voter yet), nor strong objection here on the lists. The minimal voting period is 1 week, so theoretically if it were ended on Wed (the voting RFC doesn’t disallow this) – there were still some time to do extensive testing and fixes. Alpha2 is the time where a) a lot of users will be able to test it and b) it still can be reverted in the worst case. What do you think? Regards Anatol From: Dmitry Stogov [mailto:dmi...@zend.com] Sent: Wednesday, June 10, 2015 9:37 AM To: Levi Morrison; Aaron Piotrowski; Nikita Popov Cc: internals; Anatol Belski; Kalle Sommer Nielsen Subject: Re: [PHP-DEV] [RFC] Throwable Interface On Tue, Jun 9, 2015 at 9:13 PM, Levi Morrison le...@php.net mailto:le...@php.net wrote: On Tue, Jun 9, 2015 at 11:40 AM, Aaron Piotrowski aa...@icicle.io mailto:aa...@icicle.io wrote: Does anyone have any questions, comments, or concerns about the Throwable Interface RFC? http://wiki.php.net/rfc/throwable-interface The proposed exception hierarchy: interface Throwable ⊢ Exception implements Throwable ∟ Error implements Throwable (replaces EngineException) ⊢ TypeError (replaces TypeException) ∟ ParseError (replaces ParseException) I’d like to complete a vote before too many alpha releases are made. I’ll be happy to resolve the merge conflicts in the PR if the vote passes. My only objections have already been raised, but I'll reiterate them briefly: 1. Having both Error and Exception makes little sense, especially since we have historically used error to refer to an error that wasn't an exception (something that triggered the error handler). 2. The name Error is going to have a fairly high collision rate with user code. I also like EngineException more than Error. 3. I think they should all use Exception as the root instead having a new root with multiple children (and yes, I am aware of the impact of this, and it has already been discussed on this list). This done on purpose. To prevent catching of new exceptions by old PHP code. If we won't accept this now (before 7.0 release), it would make more troubles in the future. I didn't make deep code review, and don't know all possible consequences. Anyway, I would make a chance to accept this now or never. RM, your thoughts? In case we decide to vote for this RFC, I'll make code review and will help with possible problems. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
On Jun 14, 2015, at 11:31 AM, Anatol Belski anatol@belski.net wrote: Hi Dmitry, I would go by accepting this. Furthermore – if you feel that the implementation is stable enough and does not BC, I would suggest to have it already in the alpha2. As there seems to be at all no resistance in the votes (no even single “no” voter yet), nor strong objection here on the lists. The minimal voting period is 1 week, so theoretically if it were ended on Wed (the voting RFC doesn’t disallow this) – there were still some time to do extensive testing and fixes. Alpha2 is the time where a) a lot of users will be able to test it and b) it still can be reverted in the worst case. What do you think? Regards Anatol I will have some time to resolve the merge conflicts later today, so I will be happy to take care of that. Regards, Aaron Piotrowski -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
On Tue, Jun 9, 2015 at 9:13 PM, Levi Morrison le...@php.net wrote: On Tue, Jun 9, 2015 at 11:40 AM, Aaron Piotrowski aa...@icicle.io wrote: Does anyone have any questions, comments, or concerns about the Throwable Interface RFC? http://wiki.php.net/rfc/throwable-interface The proposed exception hierarchy: interface Throwable ⊢ Exception implements Throwable ∟ Error implements Throwable (replaces EngineException) ⊢ TypeError (replaces TypeException) ∟ ParseError (replaces ParseException) I’d like to complete a vote before too many alpha releases are made. I’ll be happy to resolve the merge conflicts in the PR if the vote passes. My only objections have already been raised, but I'll reiterate them briefly: 1. Having both Error and Exception makes little sense, especially since we have historically used error to refer to an error that wasn't an exception (something that triggered the error handler). 2. The name Error is going to have a fairly high collision rate with user code. I also like EngineException more than Error. 3. I think they should all use Exception as the root instead having a new root with multiple children (and yes, I am aware of the impact of this, and it has already been discussed on this list). This done on purpose. To prevent catching of new exceptions by old PHP code. If we won't accept this now (before 7.0 release), it would make more troubles in the future. I didn't make deep code review, and don't know all possible consequences. Anyway, I would make a chance to accept this now or never. RM, your thoughts? In case we decide to vote for this RFC, I'll make code review and will help with possible problems. Thanks. Dmitry. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
3. I think they should all use Exception as the root instead having a new root with multiple children (and yes, I am aware of the impact of this, and it has already been discussed on this list). This done on purpose. To prevent catching of new exceptions by old PHP code. We have code that previously triggered error handlers or got caught by exceptions and now will not trigger the handler nor get caught. If only fatal errors had been converted this would not be an issue, but some non-fatals were converted (TypeException) and now we have this issue. Why is that less of an issue than catching *all* exceptions? (I'm seriously asking, that is not rhetorical) -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
On Jun 10, 2015, at 2:37 AM, Dmitry Stogov dmi...@zend.com wrote: I also like EngineException more than Error. I think EngineException has a couple of problems with it: 1) EngineException doesn’t really accurately represent the reason for the error. For example, passing the wrong type to a function isn’t an ‘engine' problem, it’s an error in user code. In the future it may be desirable to add more classes in this exception branch, and I think a more general name would be more desirable. 2) The name EngineException implies it descends from Exception. I worry that this will cause a lot of confusion for users thinking they have to be catching Exceptions when the problem is actually an error in their code. Discussing why you should not catch Error objects is easier than trying to explain why certain Exception objects should not be caught. Error is best choice for the name of the second exception branch. It has precedent in other languages and is less likely to cause confusion. Most user code that would collide with the name is likely very outdated, but still could easily be updated in only a few minutes. Regards, Aaron Piotrowski
Re: [PHP-DEV] [RFC] Throwable Interface
Hi! On 6/9/15 10:40 AM, Aaron Piotrowski wrote: Does anyone have any questions, comments, or concerns about the Throwable Interface RFC? http://wiki.php.net/rfc/throwable-interface The proposed exception hierarchy: interface Throwable ⊢ Exception implements Throwable ∟ Error implements Throwable (replaces EngineException) ⊢ TypeError (replaces TypeException) ∟ ParseError (replaces ParseException) Don't see anything wrong with it. I think we can put this to a vote. -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
Hello :-), At Hoa, we are totally in favor or this RFC. We tried to make our exception hierarchy compatible with PHP7 with success but it was very difficult to make it backward compatible. With this proposal, all our problems are going to be solved, so +1 for us! On 23/05/15 22:12, Aaron Piotrowski wrote: Hello, I’ve created an RFC for modifying the exception hierarchy for PHP 7, adding Throwable interface and renaming the exceptions thrown from fatal errors. The RFC is now ready for discussion. RFC: https://wiki.php.net/rfc/throwable-interface https://wiki.php.net/rfc/throwable-interface Pull Request: https://github.com/php/php-src/pull/1284 https://github.com/php/php-src/pull/1284 Aaron Piotrowski -- Ivan Enderlin Developer of Hoa http://hoa-project.net/ PhD. computer scientist Hacker http://mnt.io/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
Levi Morrison wrote on 10/06/2015 15:37: We have code that previously triggered error handlers or got caught by exceptions and now will not trigger the handler nor get caught. AFAIK, the only things which are going to not inherit from Exception were things which didn't inherit from Exception in the first place. The only edge case is E_RECOVERABLE, which previously triggered an error handler, and now will require a different mechanism (catch ( Error $e ) or catch ( Throwable $e )). some non-fatals were converted (TypeException) There was no equivalent to TypeException in previous versions of PHP, so in what sense has it been converted? Maybe I'm being thick and there's a situation that has been, but the only type-related error I know of was type-hinting classes, which were fatal. Regards, -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
some non-fatals were converted (TypeException) There was no equivalent to TypeException in previous versions of PHP, so in what sense has it been converted? Maybe I'm being thick and there's a situation that has been, but the only type-related error I know of was type-hinting classes, which were fatal. When an argument was provided that did not fulfill the parameter's type requirement an E_RECOVERABLE was emitted. This meant that the error handler was triggered. Now it will not be triggered, nor will it get caught by `catch (Exception)`. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
On Tue, Jun 9, 2015 at 11:40 AM, Aaron Piotrowski aa...@icicle.io wrote: Does anyone have any questions, comments, or concerns about the Throwable Interface RFC? http://wiki.php.net/rfc/throwable-interface The proposed exception hierarchy: interface Throwable ⊢ Exception implements Throwable ∟ Error implements Throwable (replaces EngineException) ⊢ TypeError (replaces TypeException) ∟ ParseError (replaces ParseException) I’d like to complete a vote before too many alpha releases are made. I’ll be happy to resolve the merge conflicts in the PR if the vote passes. My only objections have already been raised, but I'll reiterate them briefly: 1. Having both Error and Exception makes little sense, especially since we have historically used error to refer to an error that wasn't an exception (something that triggered the error handler). 2. The name Error is going to have a fairly high collision rate with user code. 3. I think they should all use Exception as the root instead having a new root with multiple children (and yes, I am aware of the impact of this, and it has already been discussed on this list). -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
Does anyone have any questions, comments, or concerns about the Throwable Interface RFC? http://wiki.php.net/rfc/throwable-interface The proposed exception hierarchy: interface Throwable ⊢ Exception implements Throwable ∟ Error implements Throwable (replaces EngineException) ⊢ TypeError (replaces TypeException) ∟ ParseError (replaces ParseException) I’d like to complete a vote before too many alpha releases are made. I’ll be happy to resolve the merge conflicts in the PR if the vote passes. Regards, Aaron Piotrowski -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
On Jun 9, 2015, at 1:13 PM, Levi Morrison le...@php.net wrote: My only objections have already been raised, but I'll reiterate them briefly: 1. Having both Error and Exception makes little sense, especially since we have historically used error to refer to an error that wasn't an exception (something that triggered the error handler). 2. The name Error is going to have a fairly high collision rate with user code. 3. I think they should all use Exception as the root instead having a new root with multiple children (and yes, I am aware of the impact of this, and it has already been discussed on this list). -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php http://www.php.net/unsub.php I outlined the reasons for the Error name choice in the RFC. While it may cause some confusion, I feel it is a better choice than obfuscating the different exception branches with similar class names. Having a class named TypeException that does not extend Exception is unintuitive. I think users will be confused by catch (Exception $e) not catching TypeException. Using a different suffix makes it clearer that catch (Error $e) is necessary to catch TypeError. Error will collide with some user code, but changing the name of a class to make code compatible with PHP 7 is a relatively easy change to make, largely accomplished with a find-and-replace. Hopefully most user code is using namespaces and collisions will be avoided. Suggestions are certainly welcome for a different name, but honestly I think Error makes a lot of sense. Non-fatal errors in PHP will trigger notices and warnings, while fatal errors can be thrown. Users likely will use the term ‘Uncaught Error’ when searching, minimizing the overlap with information about non-fatal errors. Perhaps it would be best if the vote was separated: 1) Replace BaseException with Throwable interface. Hierarchy otherwise remains as-is, with EngineException implementing Throwable, TypeException extending EngineException, and ParseException implementing Throwable. 2) If (1) is accepted, also change EngineException to Error, TypeException to TypeError, and ParseException with ParseError, with TypeError and ParseError extending Error. Regards, Aaron Piotrowski
Re: [PHP-DEV] [RFC] Throwable Interface
Hi all, On Tue, May 26, 2015 at 3:02 AM, Rowan Collins rowan.coll...@gmail.com wrote: On 24/05/2015 22:32, Yasuo Ohgaki wrote: Does this include internal function type errors? e.g. $ php -r 'var_dump(mt_srand(999));' PHP Warning: mt_srand() expects parameter 1 to be integer, string given in Command line code on line 1 NULL If not, please make these exceptions rather than E_WARNING. We need consistency here. Changing a Warning to an Exception is a much bigger deal than changing an Error to an Exception. While an Exception *can* be caught, the fundamental nature of it is that it is *fatal* if it is not caught. So you would be changing an advisory warning to a fatal error. There might be cases where this would be sensible, but this has nothing to do with a) changing Errors to Exceptions (which has already passed) or b) changing the hierarchy to add a Throwable interface (the subject of this thread). Error and unhandled exception differs a lot. I agree. Personally, I don't mind at all if internal function raises exception for type errors, but others may be disturbed a lot probably. We need a roadmap to clean this mess up. How about add - Change internal function type error to exception by PHP 7.1 (or PHP 7.2 if a year is not enough) to the RFC for smoother migration and future consistency? Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] [RFC] Throwable Interface
On 05/24/2015 10:11 PM, Aaron Piotrowski wrote: On May 24, 2015, at 2:08 PM, Marc Bennewitz dev@mabe.berlin wrote: Where are the new classes and the interface located if it's not in the global namespace or do I muss something? Sorry if what I wrote wasn’t clear. Throwable, Error, TypeError, and ParseError will be built-in interfaces/classes in the root namespaces. So users will not be able to make classes with those exact names, but classes with those names in a non-global namespace (e.g., \Vendor\Package\TypeError) will still be possible. I’ve updated the RFC to make this clearer. Thanks If I remember right the TypeException/TypeError will be thrown in some cases of internal functions with strict argument count. In my opinion a missing argument or to much arguments has nothing to do with the type and should have it's own Exception/Error class. I believe this actually generates a warning, it does not throw an exception. However, this does remind me of another point: It is possible that more classes extending Error could be created in the future for different error reasons. Anything that throws an Error could later be changed to throw an object extending Error without a BC break. This is a separate issue though and could be discussed in a future RFC. Ok than I remember wrong. Aaron Piotrowski Marc -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
On 05/24/2015 11:32 PM, Yasuo Ohgaki wrote: Hi Aaron, On Sun, May 24, 2015 at 5:12 AM, Aaron Piotrowski aa...@icicle.io wrote: I’ve created an RFC for modifying the exception hierarchy for PHP 7, adding Throwable interface and renaming the exceptions thrown from fatal errors. The RFC is now ready for discussion. RFC: https://wiki.php.net/rfc/throwable-interface https://wiki.php.net/rfc/throwable-interface Pull Request: https://github.com/php/php-src/pull/1284 https://github.com/php/php-src/pull/1284 Does this include internal function type errors? e.g. $ php -r 'var_dump(mt_srand(999));' PHP Warning: mt_srand() expects parameter 1 to be integer, string given in Command line code on line 1 NULL If not, please make these exceptions rather than E_WARNING. We need consistency here. This would be great for strict_types mode as there it results in a fatal error. http://3v4l.org/vHl8K Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
On 24/05/2015 22:32, Yasuo Ohgaki wrote: Does this include internal function type errors? e.g. $ php -r 'var_dump(mt_srand(999));' PHP Warning: mt_srand() expects parameter 1 to be integer, string given in Command line code on line 1 NULL If not, please make these exceptions rather than E_WARNING. We need consistency here. Changing a Warning to an Exception is a much bigger deal than changing an Error to an Exception. While an Exception *can* be caught, the fundamental nature of it is that it is *fatal* if it is not caught. So you would be changing an advisory warning to a fatal error. There might be cases where this would be sensible, but this has nothing to do with a) changing Errors to Exceptions (which has already passed) or b) changing the hierarchy to add a Throwable interface (the subject of this thread). Regards, -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
On 05/23/2015 10:12 PM, Aaron Piotrowski wrote: Hello, I’ve created an RFC for modifying the exception hierarchy for PHP 7, adding Throwable interface and renaming the exceptions thrown from fatal errors. The RFC is now ready for discussion. RFC: https://wiki.php.net/rfc/throwable-interface https://wiki.php.net/rfc/throwable-interface Pull Request: https://github.com/php/php-src/pull/1284 https://github.com/php/php-src/pull/1284 I like this RFC! It's much more intuitive as the current BaseException approve. Some small notes: Backwards Compatibility |Throwable|, |Error|, |TypeError|, and |ParseError| will no longer be available in the global namespace. Where are the new classes and the interface located if it's not in the global namespace or do I muss something? If I remember right the TypeException/TypeError will be thrown in some cases of internal functions with strict argument count. In my opinion a missing argument or to much arguments has nothing to do with the type and should have it's own Exception/Error class. Aaron Piotrowski Marc -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Throwable Interface
Hi Aaron, On Sun, May 24, 2015 at 5:12 AM, Aaron Piotrowski aa...@icicle.io wrote: I’ve created an RFC for modifying the exception hierarchy for PHP 7, adding Throwable interface and renaming the exceptions thrown from fatal errors. The RFC is now ready for discussion. RFC: https://wiki.php.net/rfc/throwable-interface https://wiki.php.net/rfc/throwable-interface Pull Request: https://github.com/php/php-src/pull/1284 https://github.com/php/php-src/pull/1284 Does this include internal function type errors? e.g. $ php -r 'var_dump(mt_srand(999));' PHP Warning: mt_srand() expects parameter 1 to be integer, string given in Command line code on line 1 NULL If not, please make these exceptions rather than E_WARNING. We need consistency here. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] [RFC] Throwable Interface
On May 24, 2015, at 2:08 PM, Marc Bennewitz dev@mabe.berlin wrote: Where are the new classes and the interface located if it's not in the global namespace or do I muss something? Sorry if what I wrote wasn’t clear. Throwable, Error, TypeError, and ParseError will be built-in interfaces/classes in the root namespaces. So users will not be able to make classes with those exact names, but classes with those names in a non-global namespace (e.g., \Vendor\Package\TypeError) will still be possible. I’ve updated the RFC to make this clearer. If I remember right the TypeException/TypeError will be thrown in some cases of internal functions with strict argument count. In my opinion a missing argument or to much arguments has nothing to do with the type and should have it's own Exception/Error class. I believe this actually generates a warning, it does not throw an exception. However, this does remind me of another point: It is possible that more classes extending Error could be created in the future for different error reasons. Anything that throws an Error could later be changed to throw an object extending Error without a BC break. This is a separate issue though and could be discussed in a future RFC. Aaron Piotrowski -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [RFC] Throwable Interface
Hello, I’ve created an RFC for modifying the exception hierarchy for PHP 7, adding Throwable interface and renaming the exceptions thrown from fatal errors. The RFC is now ready for discussion. RFC: https://wiki.php.net/rfc/throwable-interface https://wiki.php.net/rfc/throwable-interface Pull Request: https://github.com/php/php-src/pull/1284 https://github.com/php/php-src/pull/1284 Aaron Piotrowski
