Skip to content

Zend: convert op_type IS_* to an enum - #24696

Draft
Girgias wants to merge 2 commits into
php:masterfrom
Girgias:2026-10-zend_op_type-enum
Draft

Girgias wants to merge 2 commits into
php:masterfrom
Girgias:2026-10-zend_op_type-enum

Conversation

@Girgias

@Girgias Girgias commented Oct 25, 2026

Copy link
Copy Markdown
Member

No description provided.

And use the new type to improve clarity rather than just a uint8_t
Comment thread Zend/zend_compile.h
Comment on lines +154 to +156
zend_operand_type op1_type; /* IS_UNUSED, IS_CONST, IS_TMP_VAR, IS_VAR, IS_CV */
zend_operand_type op2_type; /* IS_UNUSED, IS_CONST, IS_TMP_VAR, IS_VAR, IS_CV */
zend_operand_type result_type; /* IS_UNUSED, IS_CONST, IS_TMP_VAR, IS_VAR, IS_CV */

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.

Comments can probably go now?

@ndossche

Copy link
Copy Markdown
Member

Want a hack to get rid of the warnings?

#define IS_UNUSED  IS_UNUSED
#define IS_CONST   IS_CONST
#define IS_TMP_VAR IS_TMP_VAR
#define IS_VAR     IS_VAR
#define IS_CV      IS_CV

put this after the enum definition.

Anyway, is it worth it doing these (=this PR) kinds of changes? It takes reviewer effort to check for correctness and debugging effort to resolve issues like the warnings from CI. For me, the code was already clear in the expectation of what is allowed to be passed tbh.

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