Skip to content

Conversation

trowski
Copy link
Member

@trowski trowski commented May 16, 2015

This pull request is based on the Throwable pull request by @sebastianbergmann, but it takes a slightly different approach. This is based on comments made in that pull request as well as some comments on the internals mailing list.

I'm proposing a modification to the exception hierarchy for PHP 7:

  • Throwable (abstract class)
    • Exception
      • ErrorException
      • RuntimeException
      • LogicException
      • ...
    • Error (class names up for discussion)
      • EngineError
        • TypeError
      • ParseError

BaseException has been renamed to Throwable and a new class Error extending Throwable is used for issues that previously raised errors in PHP 5. This scheme is similar to the exception hierarchy in Java.

  • catch(Exception $e) does not catch any previously uncaught exceptions.
  • catch(Error $e) can be used to catch only interpreter errors.
  • catch(Throwable $e) can catch any exception or error class.

Pros:

  • Clear separation of thrown classes. Error is only thrown due to problems that previously raised errors.
  • Error naming scheme chosen to avoid confusion. Example: why would catch (Exception $e) not catch an object named EngineException?

Cons:

  • Objects can be thrown that do not have Exception in the name.
  • BC: Existing Error classes in the global namespace would need to be renamed.

Edit: I would rather a distinction wasn't made between normal exceptions and errors, but unfortunately we probably should for BC. Exceptions thrown by the engine shouldn't directly extend Exception because they would cause BC issues, being unintentionally caught by catch (Exception $e). Since this distinction must be made, I think it's better to make the distinction clearer rather than try and obfuscate it and cause confusion for users.

trowski added 2 commits May 16, 2015 01:15
EngineException, TypeException, and ParseException
were renamed to EngineError, TypeError, and ParseError.
Each now extends Error. Exception and Error extends
Throwable, the common exception class previously called
BaseException.
@Wes1262
Copy link

Wes1262 commented May 16, 2015

Java's Exception hierarchy sucks. This is the best comment I can give. I'm sorry if it sounds harsh but it's just the truth. We definitely don't want to emulate something that is universally hated by the whole java community.

@trowski
Copy link
Member Author

trowski commented May 16, 2015

Java's Exception hierarchy sucks. This is the best comment I can give. I'm sorry if it sounds harsh but it's just the truth. We definitely don't want to emulate something that is universally hated by the whole java community.

Java's exceptions suck because of checked and unchecked exceptions. This PR does nothing to implement any craziness like that, because I agree, the last thing we want to do is emulate Java. I regret even mentioning the comparison.

All this PR does is push a new class Error between what was called BaseException and the exceptions thrown by the interpreter and rename a few things to make the naming cleaner and clearer.

@laruence laruence added RFC PHP7 and removed RFC labels May 16, 2015
@smalyshev smalyshev added the RFC label May 16, 2015
@smalyshev
Copy link
Contributor

I think Java problems mainly come from checked exceptions and the boilerplate that is involved in this. I don't see this patch having the same problems.

@trowski
Copy link
Member Author

trowski commented Jun 16, 2015

Closing as #1284 supersedes this PR.

@trowski trowski closed this Jun 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants