Skip to content

zend_api: Use return true / return false for functions returning bool#21661

Open
LamentXU123 wants to merge 2 commits intophp:masterfrom
LamentXU123:ret-tf-instead-of-10
Open

zend_api: Use return true / return false for functions returning bool#21661
LamentXU123 wants to merge 2 commits intophp:masterfrom
LamentXU123:ret-tf-instead-of-10

Conversation

@LamentXU123
Copy link
Copy Markdown
Contributor

same as #21649

Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

You missed quite a few of them.

@TimWolla
Copy link
Copy Markdown
Member

TimWolla commented Apr 7, 2026

You missed quite a few of them.

This kind of change should be done in bulk with Coccinelle and if Coccinelle doesn't catch all of them, it should be investigated why. The main reason I only did zend_compile.c in the linked PR is because I planned some work on the compiler specifically.

@LamentXU123
Copy link
Copy Markdown
Contributor Author

LamentXU123 commented Apr 8, 2026

Yea I directly use Coccinelle for this. Now, Lets change the missed case and I plan to open one PR to change these cases in bulk in the future

@TimWolla Seems like The Coccinelle script in the linked PR couldn't match cases wit different indent. e.g.
It can match:

foo();
return 0;

It can't match

foo;
    return 0;

See if this would be better instead:

@r1@
identifier fn;
typedef bool;
@@

bool fn(...)
{
...
(
- return 0;
+ return false;
|
- return 1;
+ return true;
)
...
}

@LamentXU123
Copy link
Copy Markdown
Contributor Author

Oh, also I'm curious about why we are doing these refactors (I submit this just because I saw someone else did XD) .If benefits are little I think maybe we could avoid this code churn🤔

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.

4 participants