Skip to content

refactor: Drop unused op_array param from zend_get_brk_cont_target() #19117

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

Merged
merged 3 commits into from
Jul 15, 2025

Conversation

xepozz
Copy link
Contributor

@xepozz xepozz commented Jul 13, 2025

No description provided.

int array_offset = opline->op1.num;
static uint32_t zend_get_brk_cont_target(const zend_op *opline) {
uint32_t nest_levels = opline->op2.num;
uint32_t array_offset = opline->op1.num;
Copy link
Member

Choose a reason for hiding this comment

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

Technically should be fine, but we use int and even zend_long in other places (e.g. zend_compile_break_continue(), CG(context).current_brk_cont, etc.). If this needs to be adjusted, all of them should be.

@iluuu1994
Copy link
Member

Side-note:

The head ref may contain hidden characters: "\u0441"

Your branch-name seems to be messed up.

@iluuu1994
Copy link
Member

@xepozz Can you revert the type change so we can merge the dropping of the zend_op_array *op_array parameter. If you want to, you can adjust the type in a separate PR afterwards.

@xepozz
Copy link
Contributor Author

xepozz commented Jul 15, 2025

@iluuu1994 done

@TimWolla
Copy link
Member

@xepozz I think the PR needs retitling now, the “and sync var types” part is no longer accurate, right?

@iluuu1994 iluuu1994 merged commit ea9a7b2 into php:master Jul 15, 2025
9 checks passed
@iluuu1994 iluuu1994 changed the title refactor: clean zend_get_brk_cont_target arguments and sync var types refactor: Drop unused op_array param from zend_get_brk_cont_target() Jul 15, 2025
@iluuu1994
Copy link
Member

Renamed while merging. Thanks @xepozz!

@xepozz xepozz deleted the с branch July 15, 2025 18:08
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