Improve BC support of arginfo files generated by gen_stub.php #13705
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
kocsismate
commented
Mar 13, 2024
•
kocsismate
commented
- Declared compatibility expectations of stub files are now enforced by a ZEND_STATIC_ASSERT calll at the top of arginfo files. This is hopefully useful for 3rd party extensions so that they don't accidentally include an arginfo file in a PHP version which it doesn't unsupport.
- Property registration for PHP 7 is fixed: function zend_declare_property_ex() is used again instead of zend_declare_typed_property(). This has been a regression since I added support for exposing doc comments.
- As a defensive measure, deep cloning is performed before newer features (type declarations, attributes etc.) are discarded before generating legacy arginfo files. Until now, some of the objects were forgotten to be taken care of. These omissions may have resulted in some weird bugs in theory (but probably they didn't have much impact in practice).
- PHP version related conditions inside non-legacy arginfo files used to possibly check for the 70000 version iD until now if compatibility with PHP 7.0 was declared in a stub. This was not 100% correct, since non-legacy arginfo files are only for PHP 8.0+. Now, I made sure that at least PHP version ID 80000 is used in the preprocessor conditions. The solution was a bit tricky though...
- Declared compatibility expectations of stub files are now enforced by a ZEND_STATIC_ASSERT calll at the top of arginfo files - Property registration for PHP 7 is fixed: function zend_declare_property_ex() is used again instead of zend_declare_typed_property(). This has been a regression since I added support for exposing doc comments. - As a defensive measure, deep cloning is performed before newer features (type declarations, attributes etc.) are discarded before generating legacy arginfo files. Until now, some of the objects were forgotten to be taken care of. These omissions may have resulted in some weird bugs in theory (but probably they didn't have much impact in practice). - PHP version related conditions inside *non-legacy arginfo files* used to possibly check for the 70000 version iD until now if compatibility with PHP 7.0 was declared in a stub. This was not 100% correct, since non-legacy arginfo files are only for PHP 8.0+. Now, I made sure that at least PHP version ID 80000 is used in the preprocessor conditions. The solution was a bit tricky though...
kocsismate
changed the title
kocsismate
commented
Mar 18, 2024
| @@ -1,6 +1,9 @@ | |||
| /* This is a generated file, edit the .stub.php file instead. | |||
| * Stub hash: dd3f852ea9f8e3a356ed226380edf5cc336f8a4e */ | |||
|
|
|||
| ZEND_STATIC_ASSERT(PHP_VERSION_ID >= 80000, "test_arginfo.h only supports PHP version ID 80000 or newer, " | |||
There was a problem hiding this comment.
Are you fine with added such static asserts for arginfo files when the stub file has a version constraint?
There was a problem hiding this comment.
I don't have any strong opinions about this
There was a problem hiding this comment.
OK, then I merge it, and in case anyone complains, I get rid of it or change it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.