Generate C enums from internal enums, introduce Z_PARAM_ENUM() #20917
Conversation
|
|
||
| static zend_always_inline zend_long zend_enum_fetch_case_id(zend_object *zobj) | ||
| { | ||
| ZEND_ASSERT(zobj->ce->ce_flags & ZEND_ACC_ENUM); |
There was a problem hiding this comment.
This assert is redundant with the one in zend_enum_obj_from_obj(). I also don't see how it could help with codegen.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
No particularly strong feelings either way, though.
There was a problem hiding this comment.
The _decl files should be listed in .gitattributes as linguist-generated -diff.
There was a problem hiding this comment.
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.
arnaud-lb
force-pushed
the
enum-c-enum
branch
from
aad476a to
734389e
Compare
github-actions
bot
added
Extension: pgsql
Extension: phar
Extension: posix
Extension: readline
Extension: session
Extension: shmop
Extension: simplexml
Extension: snmp
Extension: soap
Extension: sockets
Extension: sodium
Extension: spl
Extension: sqlite3
Extension: sysvmsg
Extension: sysvsem
Extension: sysvshm
Extension: tidy
Extension: tokenizer
Extension: xml
Extension: xmlreader
Extension: xmlwriter
Extension: xsl
Extension: zip
Extension: zlib
SAPI: apache2handler
SAPI: cgi
SAPI: cli
SAPI: litespeed
SAPI: phpdbg
labels
| && ($hasDeclHeader ? $stubHash === $oldStubHashDecl : $oldStubHashDecl === null); | ||
| if ($generatedFilesUpToDate && !$context->forceParse) { | ||
| /* Stub file did not change, do not regenerate. */ | ||
| return null; |
There was a problem hiding this comment.
At this point I wonder if it makes sense to just unconditionally regenerate the stub files (but keep the hash as an easy way to generate “merge conflicts”, forcing users to regenerate the arginfo). Make's mtime tracking should already ensure that arginfo files are not needlessly regenerated.
see also #20891.
There was a problem hiding this comment.
It was also non-obvious to me why the “stub hash” changed, despite the stubs not changing. Previously it was a straight-forward “stub hash”, now it's more of a “stub version” due to the _v2 suffix.
The change to the header to include the actual file name makes sense to me (but should ship independently, since it is so noisy having it in this PR).
5 participants
Update
gen_stubs.phpto generate C enums from internal enums. Enum values can be compared to the result ofzend_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.hfiles would generate warnings if we tried to include them in a compilation unit that doesn't call theregister_{$class}functions, for instance.Introduce
Z_PARAM_ENUM()(similarly to #20898).cc @TimWolla