Error Subclasses - Externals

9 years ago by Aaron Piotrowski — view source

unread

Hello everyone!

I recently pushed changes that eliminated E_EXCEPTION and allows an exception type to be provided for what were fatals in PHP, while still falling back to an E_ERROR if necessary.

Since more specific Error classes can be thrown, I'd like to propose the following additions to the Error tree of exceptions: AccessError and IdentifierError.

AccessError - Thrown when trying attempting to call a public, private, or abstract method, when statically calling a non-static method, or trying to use self::, parent::, or static:: outside of a class.
IdentifierError - Thrown when referencing an undefined function, method, class, constant, etc.

I’ve created a patch that implements the exceptions above as well as updating all the related tests: https://github.com/trowski/php-src/tree/error-subclasses https://github.com/trowski/php-src/tree/error-subclasses

This patch also broadens the usage of TypeError to include conditions such as calling a method on a scalar, passing a value that does not specify a callback when one is expected, and various other conditions based on an incorrect type that otherwise are throwing plain Error objects.

This patch introduces no functional changes, only more specific types of Errors are thrown from conditions that were already throwing Error objects.

I was hoping this could be merged before beta 1, though I’m not sure if the time table is too tight.

Cheers,
Aaron Piotrowski

9 years ago by Anatol Belski — view source

unread

Hi Aaron,

-----Original Message-----
From: Aaron Piotrowski [mailto:aaron@icicle.io]
Sent: Monday, July 6, 2015 8:16 AM
To: internals@lists.php.net
Subject: [PHP-DEV] Error Subclasses

Hello everyone!

I recently pushed changes that eliminated E_EXCEPTION and allows an
exception type to be provided for what were fatals in PHP, while still falling back
to an E_ERROR if necessary.

Since more specific Error classes can be thrown, I'd like to propose the following
additions to the Error tree of exceptions: AccessError and IdentifierError.

AccessError - Thrown when trying attempting to call a public, private, or
abstract method, when statically calling a non-static method, or trying to use
self::, parent::, or static:: outside of a class.
IdentifierError - Thrown when referencing an undefined function, method, class,
constant, etc.

I’ve created a patch that implements the exceptions above as well as updating
all the related tests: https://github.com/trowski/php-src/tree/error-subclasses
https://github.com/trowski/php-src/tree/error-subclasses

This patch also broadens the usage of TypeError to include conditions such as
calling a method on a scalar, passing a value that does not specify a callback
when one is expected, and various other conditions based on an incorrect type
that otherwise are throwing plain Error objects.

This patch introduces no functional changes, only more specific types of Errors
are thrown from conditions that were already throwing Error objects.

I was hoping this could be merged before beta 1, though I’m not sure if the time
table is too tight.

Thanks for the ping. While I find the idea about more specific exceptions correct, I would not recommend merging it in beta1. Reason - we have no big time now to verify the patch completeness, to discuss the exception names and areas where it's applicable.

IMHO if it goes in, it has to be complete and well verified, maybe also voted (regarding namings). As the area is very public and if we find any issues later and have to rename/rework the exception names, etc. - it would be bad. So 7.1 might be a better place to target. Technically it would be anyway worky as those specialized extension classes will have the same parent.

Regards

Anatol

9 years ago by Bob Weinand — view source

unread

Hey Anatol

Am 06.07.2015 um 12:06 schrieb Anatol Belski anatol.php@belski.net:

Hi Aaron,

-----Original Message-----
From: Aaron Piotrowski [mailto:aaron@icicle.io]
Sent: Monday, July 6, 2015 8:16 AM
To: internals@lists.php.net
Subject: [PHP-DEV] Error Subclasses

Hello everyone!

I recently pushed changes that eliminated E_EXCEPTION and allows an
exception type to be provided for what were fatals in PHP, while still falling back
to an E_ERROR if necessary.

Since more specific Error classes can be thrown, I'd like to propose the following
additions to the Error tree of exceptions: AccessError and IdentifierError.

AccessError - Thrown when trying attempting to call a public, private, or
abstract method, when statically calling a non-static method, or trying to use
self::, parent::, or static:: outside of a class.
IdentifierError - Thrown when referencing an undefined function, method, class,
constant, etc.

I’ve created a patch that implements the exceptions above as well as updating
all the related tests: https://github.com/trowski/php-src/tree/error-subclasses
https://github.com/trowski/php-src/tree/error-subclasses

This patch also broadens the usage of TypeError to include conditions such as
calling a method on a scalar, passing a value that does not specify a callback
when one is expected, and various other conditions based on an incorrect type
that otherwise are throwing plain Error objects.

This patch introduces no functional changes, only more specific types of Errors
are thrown from conditions that were already throwing Error objects.

I was hoping this could be merged before beta 1, though I’m not sure if the time
table is too tight.

Thanks for the ping. While I find the idea about more specific exceptions correct, I would not recommend merging it in beta1. Reason - we have no big time now to verify the patch completeness, to discuss the exception names and areas where it's applicable.

IMHO if it goes in, it has to be complete and well verified, maybe also voted (regarding namings). As the area is very public and if we find any issues later and have to rename/rework the exception names, etc. - it would be bad. So 7.1 might be a better place to target. Technically it would be anyway worky as those specialized extension classes will have the same parent.

Regards

Anatol

I like what Aaron did here.

I really think that should target 7.0. It's actually not breaking anything (really just changing the exception names).
And I really think we should have a proper Error hierarchy at the release of PHP 7.0. Considering it especially one of the features of 7.0.

But I think, at end of beta 2 or so, we really should do a final review of all the Errors in order to ensure everything is aptly named and used.

Bob

9 years ago by Anatol Belski — view source

unread

Hi Bob,

-----Original Message-----
From: Bob Weinand [mailto:bobwei9@hotmail.com]
Sent: Monday, July 6, 2015 2:59 PM
To: Anatol Belski
Cc: Aaron Piotrowski; internals@lists.php.net
Subject: Re: [PHP-DEV] Error Subclasses

Hey Anatol

Am 06.07.2015 um 12:06 schrieb Anatol Belski anatol.php@belski.net:

Hi Aaron,

-----Original Message-----
From: Aaron Piotrowski [mailto:aaron@icicle.io]
Sent: Monday, July 6, 2015 8:16 AM
To: internals@lists.php.net
Subject: [PHP-DEV] Error Subclasses

Hello everyone!

I recently pushed changes that eliminated E_EXCEPTION and allows an
exception type to be provided for what were fatals in PHP, while
still falling back to an E_ERROR if necessary.

Since more specific Error classes can be thrown, I'd like to propose
the following additions to the Error tree of exceptions: AccessError and
IdentifierError.

AccessError - Thrown when trying attempting to call a public,
private, or abstract method, when statically calling a non-static
method, or trying to use self::, parent::, or static:: outside of a class.
IdentifierError - Thrown when referencing an undefined function,
method, class, constant, etc.

I’ve created a patch that implements the exceptions above as well as
updating all the related tests:
https://github.com/trowski/php-src/tree/error-subclasses
https://github.com/trowski/php-src/tree/error-subclasses

This patch also broadens the usage of TypeError to include conditions
such as calling a method on a scalar, passing a value that does not
specify a callback when one is expected, and various other conditions
based on an incorrect type that otherwise are throwing plain Error objects.

This patch introduces no functional changes, only more specific types
of Errors are thrown from conditions that were already throwing Error
objects.

I was hoping this could be merged before beta 1, though I’m not sure
if the time table is too tight.

Thanks for the ping. While I find the idea about more specific exceptions
correct, I would not recommend merging it in beta1. Reason - we have no big
time now to verify the patch completeness, to discuss the exception names and
areas where it's applicable.

IMHO if it goes in, it has to be complete and well verified, maybe also voted
(regarding namings). As the area is very public and if we find any issues later and
have to rename/rework the exception names, etc. - it would be bad. So 7.1
might be a better place to target. Technically it would be anyway worky as those
specialized extension classes will have the same parent.

Regards

Anatol

I like what Aaron did here.

I really think that should target 7.0. It's actually not breaking anything (really just
changing the exception names).
And I really think we should have a proper Error hierarchy at the release of PHP
7.0. Considering it especially one of the features of 7.0.

But I think, at end of beta 2 or so, we really should do a final review of all the
Errors in order to ensure everything is aptly named and used.

Thanks for the feedback. That's what I said as well - the idea is correct. Unfortunately it is too late. Just to remind - Trowable was too late as well by the time no more new RFCs was allowed. But as it was important and still acceptable in the alpha phase, so there was an exception for the exception RFC. This gives a good base to for further improvements.

Now with actual subject - it's has nothing to do with the Trowable directly as it's in. However I don't think it's to decide by me, you or anyone else alone, whether the names fit good. Also in such a short period it's hard to check whether alle the needed places are touched and everything is ok. These are two main concerns preventing this to go in, IMHO.

And we cannot target beta2 because it's a feature freeze already tomorrow when beta1 is tagged. One can argue, sure ... but some when there should be a clean cut. Then we should be working on fixing bugs and stabilizing the codebase. This PR is not a bug fix. Thus, 7.1 - this well thought and good tested feature would be welcome there. Starting with the time we've branched the 7.0 dev branch (middle September), the door is popen for any good features and RFCs. Now, having what we have - Error where it is, nothing is lost, as AnotherError extends Error.

Regards

Anatol

9 years ago by Aaron Piotrowski — view source

unread

Hi Anatol,

Hi Bob,

-----Original Message-----
From: Bob Weinand [mailto:bobwei9@hotmail.com]
Sent: Monday, July 6, 2015 2:59 PM
To: Anatol Belski
Cc: Aaron Piotrowski; internals@lists.php.net
Subject: Re: [PHP-DEV] Error Subclasses

Hey Anatol

Am 06.07.2015 um 12:06 schrieb Anatol Belski anatol.php@belski.net:

Hi Aaron,

-----Original Message-----
From: Aaron Piotrowski [mailto:aaron@icicle.io]
Sent: Monday, July 6, 2015 8:16 AM
To: internals@lists.php.net
Subject: [PHP-DEV] Error Subclasses

Hello everyone!

I recently pushed changes that eliminated E_EXCEPTION and allows an
exception type to be provided for what were fatals in PHP, while
still falling back to an E_ERROR if necessary.

Since more specific Error classes can be thrown, I'd like to propose
the following additions to the Error tree of exceptions: AccessError and
IdentifierError.

AccessError - Thrown when trying attempting to call a public,
private, or abstract method, when statically calling a non-static
method, or trying to use self::, parent::, or static:: outside of a class.
IdentifierError - Thrown when referencing an undefined function,
method, class, constant, etc.

I’ve created a patch that implements the exceptions above as well as
updating all the related tests:
https://github.com/trowski/php-src/tree/error-subclasses
https://github.com/trowski/php-src/tree/error-subclasses

This patch also broadens the usage of TypeError to include conditions
such as calling a method on a scalar, passing a value that does not
specify a callback when one is expected, and various other conditions
based on an incorrect type that otherwise are throwing plain Error objects.

This patch introduces no functional changes, only more specific types
of Errors are thrown from conditions that were already throwing Error
objects.

I was hoping this could be merged before beta 1, though I’m not sure
if the time table is too tight.

Thanks for the ping. While I find the idea about more specific exceptions
correct, I would not recommend merging it in beta1. Reason - we have no big
time now to verify the patch completeness, to discuss the exception names and
areas where it's applicable.

IMHO if it goes in, it has to be complete and well verified, maybe also voted
(regarding namings). As the area is very public and if we find any issues later and
have to rename/rework the exception names, etc. - it would be bad. So 7.1
might be a better place to target. Technically it would be anyway worky as those
specialized extension classes will have the same parent.

Regards

Anatol

I like what Aaron did here.

I really think that should target 7.0. It's actually not breaking anything (really just
changing the exception names).
And I really think we should have a proper Error hierarchy at the release of PHP
7.0. Considering it especially one of the features of 7.0.

But I think, at end of beta 2 or so, we really should do a final review of all the
Errors in order to ensure everything is aptly named and used.

Thanks for the feedback. That's what I said as well - the idea is correct. Unfortunately it is too late. Just to remind - Trowable was too late as well by the time no more new RFCs was allowed. But as it was important and still acceptable in the alpha phase, so there was an exception for the exception RFC. This gives a good base to for further improvements.

Now with actual subject - it's has nothing to do with the Trowable directly as it's in. However I don't think it's to decide by me, you or anyone else alone, whether the names fit good. Also in such a short period it's hard to check whether alle the needed places are touched and everything is ok. These are two main concerns preventing this to go in, IMHO.

And we cannot target beta2 because it's a feature freeze already tomorrow when beta1 is tagged. One can argue, sure ... but some when there should be a clean cut. Then we should be working on fixing bugs and stabilizing the codebase. This PR is not a bug fix. Thus, 7.1 - this well thought and good tested feature would be welcome there. Starting with the time we've branched the 7.0 dev branch (middle September), the door is popen for any good features and RFCs. Now, having what we have - Error where it is, nothing is lost, as AnotherError extends Error.

Regards

Anatol

--

There are no functional issues with the patch, since as Bob pointed out, it is only changing the names of the exceptions thrown.

I agree that if beta 1 is to be the feature freeze then this will have to wait. There certainly is not enough time if we want to discuss naming or the situations where each type of Error is thrown.

I see no problem with waiting on this until 7.1 as long as introducing subclasses of Error and throwing those instead of a plain Error is not considered a BC break.

Regards,
Aaron

9 years ago by Pierre Joye — view source

unread

hi Aaron,

Hello everyone!

I recently pushed changes that eliminated E_EXCEPTION and allows an exception type to be provided for what were fatals in PHP, while still falling back to an E_ERROR if necessary.

Since more specific Error classes can be thrown, I'd like to propose the following additions to the Error tree of exceptions: AccessError and IdentifierError.

AccessError - Thrown when trying attempting to call a public, private, or abstract method, when statically calling a non-static method, or trying to use self::, parent::, or static:: outside of a class.
IdentifierError - Thrown when referencing an undefined function, method, class, constant, etc.

I’ve created a patch that implements the exceptions above as well as updating all the related tests: https://github.com/trowski/php-src/tree/error-subclasses https://github.com/trowski/php-src/tree/error-subclasses

This patch also broadens the usage of TypeError to include conditions such as calling a method on a scalar, passing a value that does not specify a callback when one is expected, and various other conditions based on an incorrect type that otherwise are throwing plain Error objects.

This patch introduces no functional changes, only more specific types of Errors are thrown from conditions that were already throwing Error objects.

Very good job! Thanks.

I was hoping this could be merged before beta 1, though I’m not sure if the time table is too tight.

In any case you may like to start a RFC already, no matter if which
versions it targets.

About 7.0 in general, naming and conventions are something I like to
see solved in 7.0. I do not think it affects stability in any way and
should not delay the final release date. One argument in favor of such
exceptions (if approved by RFC votes) is that dealing with such
changes or additions for the core languages in minor versions is
somehow more painful for our users.

In any case, please go for a RFC and see what gets out of it :)

Cheers,

Pierre

@pierrejoye | http://www.libgd.org

9 years ago by Niklas Keller — view source

unread

2015-07-07 2:22 GMT+02:00 Pierre Joye pierre.php@gmail.com:

hi Aaron,

Hello everyone!

I recently pushed changes that eliminated E_EXCEPTION and allows an
exception type to be provided for what were fatals in PHP, while still
falling back to an E_ERROR if necessary.

Since more specific Error classes can be thrown, I'd like to propose the
following additions to the Error tree of exceptions: AccessError and
IdentifierError.

AccessError - Thrown when trying attempting to call a public, private,
or abstract method, when statically calling a non-static method, or trying
to use self::, parent::, or static:: outside of a class.
IdentifierError - Thrown when referencing an undefined function, method,
class, constant, etc.

I’ve created a patch that implements the exceptions above as well as
updating all the related tests:
https://github.com/trowski/php-src/tree/error-subclasses <
https://github.com/trowski/php-src/tree/error-subclasses>;

This patch also broadens the usage of TypeError to include conditions
such as calling a method on a scalar, passing a value that does not specify
a callback when one is expected, and various other conditions based on an
incorrect type that otherwise are throwing plain Error objects.

This patch introduces no functional changes, only more specific types of
Errors are thrown from conditions that were already throwing Error objects.

Very good job! Thanks.

I was hoping this could be merged before beta 1, though I’m not sure if
the time table is too tight.

In any case you may like to start a RFC already, no matter if which
versions it targets.

About 7.0 in general, naming and conventions are something I like to
see solved in 7.0. I do not think it affects stability in any way and
should not delay the final release date. One argument in favor of such
exceptions (if approved by RFC votes) is that dealing with such
changes or additions for the core languages in minor versions is
somehow more painful for our users.

In any case, please go for a RFC and see what gets out of it :)

Cheers,

Pierre

@pierrejoye | http://www.libgd.org

--

I'm with Anatol here. It's no BC break if we only introduce subtypes
instead of the parent type, but it would be a BC break afterwards if the
names we choose now do not then. It feels pretty rushed to get it in before
beta1 (which is the final feature freeze).

As errors shouldn't be caught except for logging, there's also little
benefit in rushing in subtypes them now. The only benefit that I can see is
the exception name as a short and concise error description. It's nothing
like Throwable, which needed that exception, because a later introduction
would have been a BC break.

Regards, Niklas