Skip to content

ext/intl: Refactor error handling #19196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jul 20, 2025

This is a comprehensive refactoring of the error mechanism of the Intl extension.

By moving the prefixing of the current method/function being executed to actual error message creation by accessing the execution context, we get the following benefits:

  • Accurate error messages indicating what call caused the error
    • As we always "copy" the message, the copyMsg arg becomes unused, meaning we can reduce the size of the intl_error struct by 4 bytes.
    • Saving it as a zend_string means we know the length of the message
  • Remove the need to pass around a "function name" char* across multiple calls
  • Use Intl's exception mechanism to generate exceptions for constructor call
    • This removes the need for replacing the error handler
    • Which didn't do anything anyway in silent mode, which required throwing non-descriptive exceptions

@Girgias Girgias force-pushed the intl-use-exception-mechanism-rather-than-custom branch from 5c1a24b to 854f1ae Compare July 27, 2025 23:31
@Girgias Girgias marked this pull request as ready for review July 27, 2025 23:45
@Girgias Girgias requested a review from devnexen as a code owner July 27, 2025 23:45
@devnexen
Copy link
Member

devnexen commented Jul 28, 2025

I appreciate the heap allocations savings ; I ll have a better look later this week. Are you still working on it though (W.I.P) ?

@Girgias Girgias changed the title ext/intl: W.I.P.: Refactor error handling ext/intl: Refactor error handling Jul 28, 2025
@Girgias
Copy link
Member Author

Girgias commented Jul 28, 2025

I appreciate the heap allocations savings ; I ll have a better look later this week. Are you still working on it though (W.I.P) ?

Forgot to update the title of the PR, but I think I'm done modifying the C/C++ code, might work on the tests to get rid of the usage of warnings as the intl.error_level INI deprecation looks likely to pass so that it doesn't end up with a bunch of deprecations :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants