Re: [PHP-DEV] Re: parent:: forwarding
Hello All, I had a chat with Etienne and Stas on IRC to try and figure out how to approach things. We decided that its best to go with only having parent:: be forwarding for alpha1. Depending on user feedback we might revisit the topic, but for now it seems that with this solution the limitations (and all 3 approaches - never, always and parent only forwarding - have limitations where they do not do what a user might need) are apparent less often and can usually be dealt with towards the base of the inheritance tree (which means it will be less of an issue in day to day programming). Etienne will handle applying the patch and doing at least a base line summary of the decision process, so that we have this as a reference if we ever revisit the topic. I hope that everybody can live with this decision. regards, Lukas -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: parent:: forwarding
Hello, On Mon, Jul 21, 2008 at 9:48 PM, Lukas Kahwe Smith [EMAIL PROTECTED] wrote: Etienne will handle applying the patch and doing at least a base line summary of the decision process, so that we have this as a reference if we ever revisit the topic. I can't apply the patch, as it requires ZE2 karma which I don't have. I'll write a RFC for it though. Regards, -- Etienne Kneuss http://www.colder.ch Men never do evil so completely and cheerfully as when they do it from a religious conviction. -- Pascal -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: parent:: forwarding
Hi! I can't apply the patch, as it requires ZE2 karma which I don't have. I'll write a RFC for it though. Please add there a link to the newest patch, since there was a lot of variations around... -- Stanislav Malyshev, Zend Software Architect [EMAIL PROTECTED] http://www.zend.com/ (408)253-8829 MSN: [EMAIL PROTECTED] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: parent:: forwarding
Yes. I use instanceof_function() in the patch. Thanks. Dmitry. Stanislav Malyshev wrote: Hi! Note that called_scope is passed only to static methods of __parent__ class. I cannot imagine the use-case with intersecting LSBs in the same class hierarchy. Could you please write the example. OK. But how do you know if called class is parent of the current scope or not? Wouldn't you have to check all the inheritance tree for the scope up to the root for that? -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: parent:: forwarding
Hi! Note that called_scope is passed only to static methods of __parent__ class. I cannot imagine the use-case with intersecting LSBs in the same class hierarchy. Could you please write the example. OK. But how do you know if called class is parent of the current scope or not? Wouldn't you have to check all the inheritance tree for the scope up to the root for that? -- Stanislav Malyshev, Zend Software Architect [EMAIL PROTECTED] http://www.zend.com/ (408)253-8829 MSN: [EMAIL PROTECTED] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: parent:: forwarding
Hi! Yes. I use instanceof_function() in the patch. Doesn't it mean we'd have this full tree lookup on every static call? I'm not sure it's a very good idea then. -- Stanislav Malyshev, Zend Software Architect [EMAIL PROTECTED] http://www.zend.com/ (408)253-8829 MSN: [EMAIL PROTECTED] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: parent:: forwarding
Hi Stas, Note that called_scope is passed only to static methods of __parent__ class. I cannot imagine the use-case with intersecting LSBs in the same class hierarchy. Could you please write the example. Thanks. Dmitry. Stanislav Malyshev wrote: Hi! Each call to static method of parent class (it doesn't mater if it was done using parent:: or something else) assumes forwarding of called context. That would make it impossible to do non-forwarding call. Which means you can't use something like ActiveRecord pattern (which uses LSB) inside the context of another method that uses LSB too. I.e. if you got the called_scope set once, you are stuck with it forever until the topmost call ends, and you can not make another (new) LSB call inside that method. I don't think it is a good idea. It would lead to massive WTFs when you have code that perfectly working in unit tests (when you call it independently without called_scope pre-set) but breaks as soon as you call it with called_scope set (since all static calls like Foo::bar() would instantly start forwarding pre-set called_scope instead of creating a new one). If this is the way it would work, I'd recommend never using this feature - you just can't know what your Foo:: would produce anymore, since the start of the call chain can be anywhere and there's no method to ensure you get the actual class that you used in the call. Which as far as I remember was the whole point behind LSB, but somehow it is lost now. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: parent:: forwarding
Here, I'm half-agree with you. I would prefer full forwarding or not forwarding at all. Thanks. Dmitry. Stanislav Malyshev wrote: Hi! With current proposal we can call the same static method from the same context with different behavior parent::foo(), self::foo(), A::foo(). That's exactly what I said when I explained why I didn't like the idea of chainging parent::. I was told I am the only one that doesn't like parent:: to be different and everybody else just loves it to be different. Well... -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: parent:: forwarding
Hi Etienne,
I took a look into the patch and I don't like it all.
At first, I don't see any consistency there.
Why parent:: does forwarding but self::, static:: and class names don't?
At second, it's too complicated.
I would propose more consistent (from my point of view) and simpler patch.
Each call to static method of parent class (it doesn't mater if it was
done using parent:: or something else) assumes forwarding of called context.
Thanks. Dmitry.
Etienne Kneuss wrote:
Hi,
The patch was made initially by Mike Lively, I adapted it and removed
forward_static_call
http://patches.colder.ch/Zend/lsb_parent_forwarding_53.patch?markup
http://patches.colder.ch/Zend/lsb_parent_forwarding_HEAD.patch?markup
They should merge without trouble.
Regards
Index: Zend/zend_execute_API.c
===
RCS file: /repository/ZendEngine2/zend_execute_API.c,v
retrieving revision 1.331.2.20.2.24.2.42
diff -u -p -d -r1.331.2.20.2.24.2.42 zend_execute_API.c
--- Zend/zend_execute_API.c 5 Jun 2008 18:50:29 -
1.331.2.20.2.24.2.42
+++ Zend/zend_execute_API.c 8 Jul 2008 11:24:51 -
@@ -1041,8 +1041,15 @@ int zend_call_function(zend_fcall_info *
current_this = EG(This);
current_called_scope = EG(called_scope);
- if (calling_scope) {
- EG(called_scope) = calling_scope;
+
+ if (EX(object)
+ (EX(function_state).function-common.fn_flags ZEND_ACC_STATIC) ==
0) {
+ EG(called_scope) = calling_scope = Z_OBJCE_P(EX(object));
+ } else if (calling_scope) {
+ if (!EG(called_scope) ||
+ !instanceof_function(EG(called_scope), calling_scope
TSRMLS_CC)) {
+ EG(called_scope) = calling_scope;
+ }
} else if (EX(function_state).function-type != ZEND_INTERNAL_FUNCTION)
{
EG(called_scope) = NULL;
}
Index: Zend/zend_vm_def.h
===
RCS file: /repository/ZendEngine2/zend_vm_def.h,v
retrieving revision 1.59.2.29.2.48.2.58
diff -u -p -d -r1.59.2.29.2.48.2.58 zend_vm_def.h
--- Zend/zend_vm_def.h 11 Jun 2008 13:18:39 - 1.59.2.29.2.48.2.58
+++ Zend/zend_vm_def.h 8 Jul 2008 11:24:52 -
@@ -1980,7 +1980,13 @@ ZEND_VM_HANDLER(113, ZEND_INIT_STATIC_ME
EX(fbc) = ce-constructor;
}
- EX(called_scope) = ce;
+ if (EG(called_scope)
+ (EX(fbc)-common.fn_flags ZEND_ACC_STATIC) != 0
+ instanceof_function(EG(called_scope), ce TSRMLS_CC)) {
+ EX(called_scope) = EG(called_scope);
+ } else {
+ EX(called_scope) = ce;
+ }
if (EX(fbc)-common.fn_flags ZEND_ACC_STATIC) {
EX(object) = NULL;
Index: Zend/zend_vm_execute.h
===
RCS file: /repository/ZendEngine2/zend_vm_execute.h,v
retrieving revision 1.62.2.30.2.49.2.58
diff -u -p -d -r1.62.2.30.2.49.2.58 zend_vm_execute.h
--- Zend/zend_vm_execute.h 11 Jun 2008 13:18:39 -
1.62.2.30.2.49.2.58
+++ Zend/zend_vm_execute.h 8 Jul 2008 11:24:54 -
@@ -2613,7 +2613,13 @@ static int ZEND_FASTCALL ZEND_INIT_STAT
EX(fbc) = ce-constructor;
}
- EX(called_scope) = ce;
+ if (EG(called_scope)
+ (EX(fbc)-common.fn_flags ZEND_ACC_STATIC) != 0
+ instanceof_function(EG(called_scope), ce TSRMLS_CC)) {
+ EX(called_scope) = EG(called_scope);
+ } else {
+ EX(called_scope) = ce;
+ }
if (EX(fbc)-common.fn_flags ZEND_ACC_STATIC) {
EX(object) = NULL;
@@ -3186,7 +3192,13 @@ static int ZEND_FASTCALL ZEND_INIT_STAT
EX(fbc) = ce-constructor;
}
- EX(called_scope) = ce;
+ if (EG(called_scope)
+ (EX(fbc)-common.fn_flags ZEND_ACC_STATIC) != 0
+ instanceof_function(EG(called_scope), ce TSRMLS_CC)) {
+ EX(called_scope) = EG(called_scope);
+ } else {
+ EX(called_scope) = ce;
+ }
if (EX(fbc)-common.fn_flags ZEND_ACC_STATIC) {
EX(object) = NULL;
@@ -3654,7 +3666,13 @@ static int ZEND_FASTCALL ZEND_INIT_STAT
EX(fbc) = ce-constructor;
}
- EX(called_scope) = ce;
+ if (EG(called_scope)
+ (EX(fbc)-common.fn_flags ZEND_ACC_STATIC) != 0
+ instanceof_function(EG(called_scope), ce TSRMLS_CC)) {
+ EX(called_scope) = EG(called_scope);
+ } else {
+ EX(called_scope) = ce;
+ }
if (EX(fbc)-common.fn_flags ZEND_ACC_STATIC) {
EX(object) = NULL;
@@ -3878,7 +3896,13 @@ static int ZEND_FASTCALL ZEND_INIT_STAT
EX(fbc) = ce-constructor;
}
- EX(called_scope) = ce;
+ if (EG(called_scope)
+ (EX(fbc)-common.fn_flags
Re: [PHP-DEV] Re: parent:: forwarding
On Tue, 8 Jul 2008, Dmitry Stogov wrote: I took a look into the patch and I don't like it all. At first, I don't see any consistency there. Why parent:: does forwarding but self::, static:: and class names don't? At second, it's too complicated. I would propose more consistent (from my point of view) and simpler patch. Each call to static method of parent class (it doesn't mater if it was done using parent:: or something else) assumes forwarding of called context. That sounds like a massive BC break to me, as that's not what happens now. regards, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: parent:: forwarding
5.2 didn't have static:: so it cannot be a BC break. Thanks. Dmitry. Derick Rethans wrote: On Tue, 8 Jul 2008, Dmitry Stogov wrote: I took a look into the patch and I don't like it all. At first, I don't see any consistency there. Why parent:: does forwarding but self::, static:: and class names don't? At second, it's too complicated. I would propose more consistent (from my point of view) and simpler patch. Each call to static method of parent class (it doesn't mater if it was done using parent:: or something else) assumes forwarding of called context. That sounds like a massive BC break to me, as that's not what happens now. regards, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: parent:: forwarding
On Tue, Jul 8, 2008 at 3:39 PM, Dmitry Stogov [EMAIL PROTECTED] wrote: I took a look into the patch and I don't like it all. At first, I don't see any consistency there. Why parent:: does forwarding but self::, static:: and class names don't? class names don't because, people expect their results to be context-independent. It is just a call of some method on some class. parent, on the other hand, implies inheritance. self and static should do forwarding too, I think, because they are context-dependent by design. forwardind is expected here At second, it's too complicated. I would propose more consistent (from my point of view) and simpler patch. Each call to static method of parent class (it doesn't mater if it was done using parent:: or something else) assumes forwarding of called context. Thanks. Dmitry. Etienne Kneuss wrote: Hi, The patch was made initially by Mike Lively, I adapted it and removed forward_static_call http://patches.colder.ch/Zend/lsb_parent_forwarding_53.patch?markup http://patches.colder.ch/Zend/lsb_parent_forwarding_HEAD.patch?markup They should merge without trouble. -- Alexey Zakhlestin http://blog.milkfarmsoft.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: parent:: forwarding
Class names are already context-dependent.
?php
class Foo {
function test() {
var_dump($this);
}
}
class Bar extends Foo {
function test() {
Foo::test(); /* this is a dynamic method call */
}
}
$x = new Bar();
$x-test();
?
Thanks. Dmitry.
Alexey Zakhlestin wrote:
On Tue, Jul 8, 2008 at 3:39 PM, Dmitry Stogov [EMAIL PROTECTED] wrote:
I took a look into the patch and I don't like it all.
At first, I don't see any consistency there.
Why parent:: does forwarding but self::, static:: and class names don't?
class names don't because, people expect their results to be
context-independent. It is just a call of some method on some
class.
parent, on the other hand, implies inheritance. self and static
should do forwarding too, I think, because they are context-dependent
by design. forwardind is expected here
At second, it's too complicated.
I would propose more consistent (from my point of view) and simpler patch.
Each call to static method of parent class (it doesn't mater if it was
done using parent:: or something else) assumes forwarding of called context.
Thanks. Dmitry.
Etienne Kneuss wrote:
Hi,
The patch was made initially by Mike Lively, I adapted it and removed
forward_static_call
http://patches.colder.ch/Zend/lsb_parent_forwarding_53.patch?markup
http://patches.colder.ch/Zend/lsb_parent_forwarding_HEAD.patch?markup
They should merge without trouble.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: parent:: forwarding
-- Forwarded message -- From: Mike Lively [EMAIL PROTECTED] Date: Tue, Jul 8, 2008 at 6:23 AM Subject: Re: [PHP-DEV] Re: parent:: forwarding To: Dmitry Stogov [EMAIL PROTECTED] On Tue, Jul 8, 2008 at 4:39 AM, Dmitry Stogov [EMAIL PROTECTED] wrote: Each call to static method of parent class (it doesn't mater if it was done using parent:: or something else) assumes forwarding of called context. Isn't this what the original LSB patches did and it was determined that letting everything forward was a bad idea? http://marc.info/?l=php-internalsm=118995456612295w=2 The idea now is that we need a combination. static class names don't forward called context but parent:: does. If I could only choose between always forwarding called context and never forwarding called context (for parent:: and class names) I would rather always forward so this wouldn't be the end of the world for me, but I do believe the mixture is a more flexible way of handling things. On a different note, is that functionality introduced by the patch you don't like or is it the specific implementation? Is there a better way to implement it and retain the functionality it introduces? (for my own benefit) -Mike Lively
Re: [PHP-DEV] Re: parent:: forwarding
Hi Mike, Yes. It was the initial proposal that was quite consistent. With current proposal we can call the same static method from the same context with different behavior parent::foo(), self::foo(), A::foo(). I didn't get why can I call parent's method, but cannot call self or grand-parent's method (with the same behavior). Thanks. Dmitry. Mike Lively wrote: -- Forwarded message -- From: Mike Lively [EMAIL PROTECTED] Date: Tue, Jul 8, 2008 at 6:23 AM Subject: Re: [PHP-DEV] Re: parent:: forwarding To: Dmitry Stogov [EMAIL PROTECTED] On Tue, Jul 8, 2008 at 4:39 AM, Dmitry Stogov [EMAIL PROTECTED] wrote: Each call to static method of parent class (it doesn't mater if it was done using parent:: or something else) assumes forwarding of called context. Isn't this what the original LSB patches did and it was determined that letting everything forward was a bad idea? http://marc.info/?l=php-internalsm=118995456612295w=2 The idea now is that we need a combination. static class names don't forward called context but parent:: does. If I could only choose between always forwarding called context and never forwarding called context (for parent:: and class names) I would rather always forward so this wouldn't be the end of the world for me, but I do believe the mixture is a more flexible way of handling things. On a different note, is that functionality introduced by the patch you don't like or is it the specific implementation? Is there a better way to implement it and retain the functionality it introduces? (for my own benefit) -Mike Lively -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: parent:: forwarding
Hi, I wouldn't mind if we weighted the pros/cons of the current solution against the initial proposition now that we had nearly 1 year to experience with the way it is now. I really feel that the current way is something that we will regret later. And as of which solution to adopt, either always-forward or parent::/self:: forward, I believe they're both acceptable. Regards On Tue, Jul 8, 2008 at 4:17 PM, Dmitry Stogov [EMAIL PROTECTED] wrote: Hi Mike, Yes. It was the initial proposal that was quite consistent. With current proposal we can call the same static method from the same context with different behavior parent::foo(), self::foo(), A::foo(). I didn't get why can I call parent's method, but cannot call self or grand-parent's method (with the same behavior). Thanks. Dmitry. Mike Lively wrote: -- Forwarded message -- From: Mike Lively [EMAIL PROTECTED] Date: Tue, Jul 8, 2008 at 6:23 AM Subject: Re: [PHP-DEV] Re: parent:: forwarding To: Dmitry Stogov [EMAIL PROTECTED] On Tue, Jul 8, 2008 at 4:39 AM, Dmitry Stogov [EMAIL PROTECTED] wrote: Each call to static method of parent class (it doesn't mater if it was done using parent:: or something else) assumes forwarding of called context. Isn't this what the original LSB patches did and it was determined that letting everything forward was a bad idea? http://marc.info/?l=php-internalsm=118995456612295w=2 The idea now is that we need a combination. static class names don't forward called context but parent:: does. If I could only choose between always forwarding called context and never forwarding called context (for parent:: and class names) I would rather always forward so this wouldn't be the end of the world for me, but I do believe the mixture is a more flexible way of handling things. On a different note, is that functionality introduced by the patch you don't like or is it the specific implementation? Is there a better way to implement it and retain the functionality it introduces? (for my own benefit) -Mike Lively -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- Etienne Kneuss http://www.colder.ch Men never do evil so completely and cheerfully as when they do it from a religious conviction. -- Pascal -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: parent:: forwarding
Hi! Each call to static method of parent class (it doesn't mater if it was done using parent:: or something else) assumes forwarding of called context. That would make it impossible to do non-forwarding call. Which means you can't use something like ActiveRecord pattern (which uses LSB) inside the context of another method that uses LSB too. I.e. if you got the called_scope set once, you are stuck with it forever until the topmost call ends, and you can not make another (new) LSB call inside that method. I don't think it is a good idea. It would lead to massive WTFs when you have code that perfectly working in unit tests (when you call it independently without called_scope pre-set) but breaks as soon as you call it with called_scope set (since all static calls like Foo::bar() would instantly start forwarding pre-set called_scope instead of creating a new one). If this is the way it would work, I'd recommend never using this feature - you just can't know what your Foo:: would produce anymore, since the start of the call chain can be anywhere and there's no method to ensure you get the actual class that you used in the call. Which as far as I remember was the whole point behind LSB, but somehow it is lost now. -- Stanislav Malyshev, Zend Software Architect [EMAIL PROTECTED] http://www.zend.com/ (408)253-8829 MSN: [EMAIL PROTECTED] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: parent:: forwarding
Hi! With current proposal we can call the same static method from the same context with different behavior parent::foo(), self::foo(), A::foo(). That's exactly what I said when I explained why I didn't like the idea of chainging parent::. I was told I am the only one that doesn't like parent:: to be different and everybody else just loves it to be different. Well... -- Stanislav Malyshev, Zend Software Architect [EMAIL PROTECTED] http://www.zend.com/ (408)253-8829 MSN: [EMAIL PROTECTED] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
