Skip to content

Remove param argument from php_verror - #22865

Merged
NattyNarwhal merged 6 commits into
php:masterfrom
NattyNarwhal:remove-params-verror
Jul 24, 2026
Merged

Remove param argument from php_verror#22865
NattyNarwhal merged 6 commits into
php:masterfrom
NattyNarwhal:remove-params-verror

Conversation

@NattyNarwhal

@NattyNarwhal NattyNarwhal commented Jul 22, 2026

Copy link
Copy Markdown
Member

Update tests and UPGRADING.INTERNALS.

Note there is some impact to extensions if they call this; however, it's usually in a centralized wrapper function which is easy to update. In my quick skim of external extensions, apcu is the only maintained one that'll need an update.

Also note that php_verror is a bit weirdly named. It should probably be called something like php_docref_verror, since php_error_docref the vararg wrapper around it (now that 1/2 are gone); let me know if this is something worth pursuing. There is php_error, which is just an alias for zend_error, which doesn't take a docref, nor does it have the function and args prefix that php_verror has. It should be used for engine stuff, but I think there are a few places where it's called from places where php_error_docref should be used instead in extensions. I can take a look at that too.

Now that docref has been knocked out and error_include_args exists,
remove this argument. Callers will be updated accordingly.
@NattyNarwhal
NattyNarwhal force-pushed the remove-params-verror branch from 78933b4 to 759a1a4 Compare July 22, 2026 21:36

@Girgias Girgias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removing php_error and recommend the docref version instead makes sense.

@NattyNarwhal

NattyNarwhal commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

I think removing php_error and recommend the docref version instead makes sense.

Just to be clear, just the alias php_error? Since zend_error is used pretty commonly throughout the engine has different output than php_verror. (Though there's also some zend_error calls in extensions too, I realize...)

@Girgias

Girgias commented Jul 23, 2026

Copy link
Copy Markdown
Member

I think removing php_error and recommend the docref version instead makes sense.

Just to be clear, just the alias php_error? Since zend_error is used pretty commonly throughout the engine has different output than php_verror. (Though there's also some zend_error calls in extensions too, I realize...)

Yeah just the alias, I think there is a way to set php_docref to be used within "zend" as a lot of utility functions that are used by zend are actually defined in main/ but that's definitely something else.

@NattyNarwhal

Copy link
Copy Markdown
Member Author

Cool, I'll try to tackle php_error/zend_error cleanup in a separate PR.

@NattyNarwhal
NattyNarwhal merged commit 0a12b3e into php:master Jul 24, 2026
18 checks passed
andypost added a commit to andypost/apcu that referenced this pull request Jul 24, 2026
php-src commit 0a12b3e8268 (php/php-src#22865) removed the docref params
argument from php_verror(), changing its signature from 5 to 4 arguments.
Since a PHP_VERSION_ID guard cannot distinguish 8.6-dev snapshots taken
before and after that change, format the message with vspprintf() and
report it through php_error_docref(), whose signature is stable across
all supported versions. Passing NULL docref and empty params to
php_verror() was equivalent to php_error_docref(NULL, ...) anyway.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JNDQDyaiHPPMtsno9Kyo4t
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.

3 participants