Skip to content

Conversation

@arnaud-lb
Copy link
Member

Update gen_stubs.php to generate C enums from internal enums. Enum values can be compared to the result of zend_enum_fetch_case_id(zend_object*).

The generated enums are added to separate files named {$extensionName}_decl.h (one for each extension declaring some enums), so that it's possible to include these from anywhere. _arginfo.h files would generate warnings if we tried to include them in a compilation unit that doesn't call the register_{$class} functions, for instance.

Introduce Z_PARAM_ENUM() (similarly to #20898).

cc @TimWolla

Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

Some first comments. Will take another look once rebased after the merge of #20915.


static zend_always_inline zend_long zend_enum_fetch_case_id(zend_object *zobj)
{
ZEND_ASSERT(zobj->ce->ce_flags & ZEND_ACC_ENUM);
Copy link
Member

Choose a reason for hiding this comment

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

This assert is redundant with the one in zend_enum_obj_from_obj(). I also don't see how it could help with codegen.

Copy link
Member Author

Choose a reason for hiding this comment

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

I though about this when adding this assert, but if we consider functions as opaque APIs, we don't know that zend_enum_obj_from_obj() has redundant asserts.

Copy link
Member

Choose a reason for hiding this comment

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

That is fair, but in this case it is probably reasonable to expect zend_enum_obj_from_obj() to do the verification (if necessary), since that's the purpose of the function, especially since a failed assert is always a programmer error.

Copy link
Member

Choose a reason for hiding this comment

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

No particularly strong feelings either way, though.

Copy link
Member

Choose a reason for hiding this comment

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

The _decl files should be listed in .gitattributes as linguist-generated -diff.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I've added **/*_decl.h, but this could accidentally match unrelated files. I'm hesitating to use another name that would be less likely to match unrelated files, such as {$ext}_arginfo_decl.h.

@iluuu1994
Copy link
Member

Nice approach. How will ADTs be handled? #define Z_PARAM_ENUM_EX(dest, obj, _ce) comes to mind. That should probably work.

@arnaud-lb
Copy link
Member Author

@iluuu1994 yes this should work. Alternatively it's possible to use Z_PARAM_OBJ_OF_CLASS() + zend_enum_fetch_case_id().

The compiler should now be able to tell us if we forget one case
@TimWolla TimWolla self-requested a review January 13, 2026 16:14
# include "php.h"
# include "php_random_csprng.h"
# include "php_random_uint128.h"
# include "random_decl.h"
Copy link
Member

Choose a reason for hiding this comment

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

Should the generated headername maybe be prefixed with php_ ?

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