adds support for DNF types in internal functions and properties #11969
adds support for DNF types in internal functions and properties #11969Girgias merged 14 commits intophp:masterfrom
Conversation
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
Overall, the approach makes sense!
But I do think having a zend_string* name be mutually exclusive with a char* name is better and less error prone in the long run even if it means possibly some more intrusive work atm
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
There is only one mistake I can spot currently, maybe add a union type to the ZendTestClass stub to pick that one up?
ext/zend_test/test.c
Outdated
| // we have to perform type-checking to avoid arginfo/zpp mismatch error, | ||
| bool type_matches = ( | ||
| instanceof_function(obj->ce, zend_ce_iterator) | ||
| || ( | ||
| instanceof_function(obj->ce, zend_ce_traversable) | ||
| && instanceof_function(obj->ce, zend_ce_countable) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
I wonder if it makes sense to provide an API to parse a param from a zend_type, but that's unrelated to this. :)
This comment was marked as resolved.
This comment was marked as resolved.
Right, so we need to add a method which takes a union of two classes then :) |
This comment was marked as resolved.
This comment was marked as resolved.
|
Ilija found where properties types were not being properly copied, and I have amended #11961 So it might make more sense for me to clean-up that PR for PHP 8.2, and then rebase on top of that fix to get rid of the weird hacks? |
Sure, I'll happily rebase and remove these memory cleanup workarounds! |
|
Fixes are merged |
Not that this does not add support for items generated by gen_stubs, only for items registered dynamically via the zend API. Closes php#10120
This allows tests to pass ASAN
ju1ius
force-pushed
the
ju1ius/internal-dnf-types
branch
from
63d2b67 to
d948c74
Compare
Doesn't matter much in practice but we are good citizens.
|
Thanks @Girgias ! 🎉 |
3 participants
DISCLAIMER: this PR does not add support for DNF types codegen in
gen_stubs.php.Currently, the routines for registering functions/methods and typed properties make assertions that cause valid
zend_types to be incorrectly rejected (see #10120).These assertions are related to the current limitations of the
gen_stubs.phpscript.This results in the zend API having a de-facto hard-dependency on the the
gen_stubs.phpcodegen tool, thus rendering it unusable when using said tool is not possible or desirable (for example when writing extensions in languages like Rust or Zig that have native C-FFI support but cannot make use of complex preprocessor macros).This pull-requests lifts those limitations while keeping
gen_stub.php-specific behaviour.Closes #10120.
Note that tests fail under ASAN because we're hitting #11883 in the added test cases.