Add Asymmetric Visibility flags #250
Conversation
Added in d04a1d6 |
ast.c
Outdated
| @@ -117,6 +117,16 @@ | |||
| # define ZEND_ENCAPS_VAR_DOLLAR_CURLY_VAR_VAR (1<<1) | |||
| #endif | |||
|
|
|||
| #if PHP_VERSION_ID < 80400 | |||
| # define MODIFIER_PUBLIC_SET (1 << 10) | |||
There was a problem hiding this comment.
Seems like in PHP 7.3 ZEND_ACC_PRIVATE was 1024: https://github.com/php/php-src/blob/89dc78e0f0c1a4f27b889f232b109a3919ecf478/Zend/zend_compile.h#L214. But for all later versions it was updated to 4: https://github.com/php/php-src/blob/master/Zend/zend_compile.h#L220C9-L220C25.
Because of that introducing MODIFIER_PUBLIC_SET for 7.2./7.3 leads to failed tests. Is there a simple way to introduce modifiers from this PR only for PHP8.4+?
There was a problem hiding this comment.
@nikic seems like setting 0 for the new flags (<8.4) fixes this. Can you please give it a look? And run tests one more time? I`ve run 7.3 and 8.4 tests locally, and it seemed fine.
There was a problem hiding this comment.
I don't think it's a good idea to set these to zero. People can reasonably expect that the flags are all disjoint and non-zero. We should find values here that work (but don't have to match any specific PHP version).
There was a problem hiding this comment.
Sorry, I probably should be more clear in my initial comment. Please let me try doing better job at explaining my vision of the problem here:
- Until 7.3
1024value was used forZEND_ACC_PRIVATE, which is registered asMODIFIER_PRIVATE - In 7.4 value of
ZEND_ACC_PRIVATEwas changed from1024to4, and after 7.4 there were no any flags with1024value. - In 8.4 Asymmetric visibility v2 was introduced. And it introduced new
ZEND_ACC_PUBLIC_SETflag with1024value.
Because of that, we can't use 1024 value for MODIFIER_PUBLIC_SET for <8.4 versions. As it conflicts with ZEND_ACC_PRIVATE flag on <7.4.
Setting 0 value for MODIFIER_PUBLIC_SET flag for <8.4 seemed as simplest approach. As this flag (and other set modifiers) is not available in those PHP versions at all.
Seems like you are suggesting to use non-zero but not-conflicting (non-existing) flag values for set modifiers for versions where they were unavailable?
|
Dear @nikic could you please review the latest updates in this PR? Are there any other changes you would like to be included here? |
|
@nikic thank you very much for merging this! Can you please release a new version that includes updates from this PR? |
Introduces new asymmetric visibility flags:
MODIFIER_PUBLIC_SET,MODIFIER_PROTECTED_SETandMODIFIER_PRIVATE_SET.