Skip to content

ext/standard: declare true as the return type of register_tick_function() - #23432

Open
lacatoire wants to merge 1 commit into
php:masterfrom
lacatoire:fix/register-tick-function-true
Open

ext/standard: declare true as the return type of register_tick_function()#23432
lacatoire wants to merge 1 commit into
php:masterfrom
lacatoire:fix/register-tick-function-true

Conversation

@lacatoire

Copy link
Copy Markdown
Member

register_tick_function() is declared bool but has no failure path beyond the ZPP step, which throws. php_add_tick_function() and zend_llist_add_element() are both void, so no status is being dropped. true is the only value ever returned.

unregister_tick_function() is already void, and ksort(), asort(), and natsort() in the same stub already declare true, so this follows existing practice.

Changes: basic_functions.stub.php (booltrue), regenerated arginfo, added test.

@lacatoire
lacatoire force-pushed the fix/register-tick-function-true branch 2 times, most recently from 14cff0b to 218d081 Compare August 24, 2026 07:28
@LamentXU123

LamentXU123 commented Aug 24, 2026

Copy link
Copy Markdown
Member

Also seems true. But I also don't think this needs a test. This is quite an improvement rather then a bug and we don't need regression test for that.
@lacatoire Could you please document this in UPGRADING as what's done before, also for #23433? This should also appears in NEWS but since the former PRs didn't do that I will do that in bulk since this is merged. Thanks!

@TimWolla

Copy link
Copy Markdown
Member

don't think this needs a test

Agreed. The test can't really test that true is the only possible return value anyways (and mismatches in debug builds will be caught anyways by other tests).

…on()

The function either throws on an invalid callback or appends the entry
to the tick list and returns true; php_add_tick_function() returns void,
so there is nothing that could report a failure.
@lacatoire
lacatoire force-pushed the fix/register-tick-function-true branch from 218d081 to 186efd3 Compare August 24, 2026 08:06
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