At 16:48 07.12.2002, Zeev Suraski wrote:
At 17:02 07/12/2002, Marcus Börger wrote:Hi Zeev,
I have changed the test files and encountered some problems with the way you modified my patch:
1) private_002.phpt fails with
004- Fatal error: Call to private method pass::show() from context 'fail' in %s on line %d
004+ Fatal error: Call to public method fail::show() from context 'fail' in /usr/src/php4-HEAD/tests/classes/private_002.php on line 18
I see the problem. It should be fixed.
Fixed now, thanks.
However this is about what i did in my first patches and gave up after discussion with Andi.The new error message is wrong. We are trying to access pass::show and not fail::show().I implement PPP in a different way altogether (it's very loosely based on your patch, the actual implementation is very different), in order to avoid having two hash lookups for every method call. I left a comment in the code that says we may want to improve the error messages, but the current implementation is quite intentional - it's much more efficient.
2) private_007.phpt and private_007b.phpt both fail with the message
Fatal error: Cannot redeclare private bar::priv() as public foo::priv()
That means the children know about their parents privates what is in contrast to
our discussion. The reason they do is because you skipped the part of the patch
where in zend_do_inheritance() a modified version of zend_hash_merge_ex() was
used to prevent this. But you also do not search for the private method in the calling
scope. In other words you mainly used the first version of my patch. So i tried to
strip down zend_hash_merge_with_argument_if() to the portion we need and tried
to skip copying zje privates. But since privates are no longer looked up in the
calling scope this fails.
Since he was right that the performance loss by the required checks is outweighted by the
fact that otherwise classes know about private details of their ancestors. And i remeber having
an agreement on this. And yes you did the execute part in another way but until now i see
many problems in your way.
3) You left in some snippets of the 'final' patch. Does this mean we are going to haveNo, we're not going to add final, at least not for now. I may have forgotten to remove the scanner part of the patch but it's meaningless :)
final? A new patch is also available.
Zeev
After the patches today some more things need to be revisited:
4) You allow changing the visibility. In most languages it is only allowed to
decrease the visibility or it is even disallowed. Increasing the visibility is a
very unfamiliar feature.
5) When you look at test private_007b.phpt you will see that the wrong
method is called. This is a consequence of 2. It seems you optimized to
much.
6) zend_execute.c, line 2350 should be
if (!(EX(fbc)->op_array.fn_flags & (ZEND_ACC_PROTECTED | ZEND_ACC_PRIVATE))) {
instead of
if (EX(fbc)->op_array.fn_flags & ZEND_ACC_PUBLIC) {
as we allow (fn_flags & ZEND_FN_PPP_MASK) == 0 being matched to public as well.
marcus
--
PHP Development Mailing List <http://www.php.net/>
To unsubscribe, visit: http://www.php.net/unsub.php
