Greetings,

A week or so ago, G.P.B. submitted a pull request that contained a declare that would automatically elevate certain errors to Error exceptions (https://github.com/php/php-src/pull/4549).

As part of the discussion, Nikic stated the following:

"I believe it's okay to convert this kind of error condition to an Error unconditionally (I've been doing this occasionally). Basically any error condition that indicates a programming error and no reasonable code should handle locally."

Since then, G.P.B. and I have independently started looking at various extensions to see what the scale of the task is to convert as many errors as possible, and you might have noticed the PR's starting to pile up.

Before I go any further with this, and because I'm brand new to contributing to php-src, I think we need to discuss if we're going to need an API to handle these changes, particularly in respect to consistent formatting of error messages and error types.

As mentioned in the various PRs, there's a several situations to consider.

1) Invalid parameters where ZPP accepts mixed but internally it requires a specific type (example: str_replace).

2) Invalid parameters where ZPP passes but there are additional constraints on the value of those parameters (example: hash_xxx with non-cryptographic hashes).

3) Errors as the result of global state (example: session changes when a session is active).

For 1) and 2) there is additionally the option of where the parameter index is known (inside PHP_FUNCTION or passing INTERNAL_FUNCTION_PARAMETERS) and where it is not (where the error occurs within a helper function).

For my own contributions, I have added a helper function php_error_parameter_validation(NULL, arg_index, format, ...) that will format a consistent message based on its argument number and the formatted text.

I've got it currently throwing Error exceptions but I can forsee replacing certain ones with TypeError when dealing with option 1.

Before I continue, can we have a quick discussion about any helpers or APIs that we may want to make this more consistent, and then we can whip up a PR that we can then use as the basis for the rest of our changes.

Also:
As part of that PR I would also like to include a universal test script called trycatch_dump(...) that accepts a list of closures and runs each one in turn, either printing the var_dump of the result, or printing the error message of the exception, prefixed with the exception name.

For test cases, which are going to be the biggest part of all this, it reduces a 5 line try-catch per statement down to:

trycatch_dump(
  fn() => func_to_test("hello"),
  fn() => func_to_test(1234),
  fn() => func_to_test(false)
);

I am however, unsure where this script should be placed in the codebase.

Mark Randall

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to