Fixed bug #69196 (PharData should extends Phar) #1157
Conversation
Doc: http://php.net/manual/en/class.phardata.php clearly declared that PharData extends Phar.
|
I am going to merge this if no objections. |
|
please hold for one or two days, before someone know this fields really see.. |
|
OK~~ |
|
Can one of the admins verify this patch? |
| @@ -5239,6 +5239,41 @@ ZEND_END_ARG_INFO() | |||
|
|
|||
| #endif /* HAVE_SPL */ | |||
|
|
|||
| /* PharData share the implement of Phar class, override are implemented inside those methods */ | |||
There was a problem hiding this comment.
so why you need to copy the whole function table? I'm not sure I understand.
There was a problem hiding this comment.
Hi @smalyshev, It is not the whole function table, this function table contains only functions for PharData class. (from the doc: http://php.net/phardata), those method internally handle different classes like this https://github.com/php/php-src/blob/master/ext/phar/phar_object.c#L1126 .
| @@ -5370,8 +5405,8 @@ void phar_object_init(TSRMLS_D) /* {{{ */ | |||
| phar_ce_archive = zend_register_internal_class(&ce TSRMLS_CC); | |||
| phar_ce_archive->ce_flags |= ZEND_ACC_FINAL_CLASS; | |||
There was a problem hiding this comment.
I'm not sure it's a good practice to extend a final class. In general, if we extend it maybe it should not be final, but if we need it to be final, maybe set it to final after we extend it.
|
Since this targets an unsupported branch, and a patch against a supported branch would have to look different, in addition there seems to be unresolved problems or questions about the implementation (asked by the author himself), I'm closing this PR. Please take this action as encouragement to open a clean PR against a supported branch. |
Doc: http://php.net/manual/en/class.phardata.php
clearly declared that PharData extends Phar.