# [patch] Callbacks bug/change request - Thread: https://externals.io/message/29953 - Original archive: https://news-web.php.net/php.internals/29953 - Started by Etienne Kneuss on 2007-05-30T14:38:35+00:00 - Messages: 15 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #29953 — Etienne Kneuss — 2007-05-30T14:38:35+00:00 URL: https://externals.io/message/29953#29953 Hi internals, It seems that there is a bug in the way some callbacks are handled. For instance: array('B', 'parent::who'); Here, the classname 'B' will only be used for inheritance checks with the current scope, but the actual resolution will be made using the current scope, and 'parent::who'. According to Marcus, this is wrong, so here is two patches (+ tests) meant for HEAD [1] and the 5_2 branch (>5.2.3) [2]. Thanks for your consideration. Regards 1: http://patches.colder.ch/Zend/callbacks_HEAD.patch?markup 2: http://patches.colder.ch/Zend/callbacks_5_2.patch?markup -- Etienne Kneuss http://www.colder.ch colder@php.net Men never do evil so completely and cheerfully as when they do it from a religious conviction. -- Pascal --- ## #29958 — Stanislav Malyshev — 2007-05-30T17:26:01+00:00 URL: https://externals.io/message/29953#29958 · In reply to: #29953 > It seems that there is a bug in the way some callbacks are handled. For > instance: > > array('B', 'parent::who'); I'm not sure such thing should be even supported. I.e. I'd expect it to try and call method named 'parent::who' in class B, which would fail since you can't define such method name. Is there any reason why anybody would really need anything like this? > According to Marcus, this is wrong, so here is two patches (+ tests) > meant for HEAD [1] > and the 5_2 branch (>5.2.3) [2]. > > Thanks for your consideration. First part of the patch changes EG(scope) even though no actual call is made. I don't think you should change globals if you are not intending to use them. If some value is needed which equals to EG(scope) or other value depending on context, I think you should do it with variable, not change the global, if possible. -- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/ --- ## #29960 — Etienne Kneuss — 2007-05-30T17:56:22+00:00 URL: https://externals.io/message/29953#29960 · In reply to: #29958 Stanislav Malyshev wrote: > > […] > > I'm not sure such thing should be even supported. I.e. I'd expect it > to try and call method named 'parent::who' in class B, which would > fail since you can't define such method name. Is there any reason why > anybody would really need anything like this? It's already supported, but buggy right now. I don't believe fixing it for odd cases will bother anyone. > > > […] > > First part of the patch changes EG(scope) even though no actual call > is made. I don't think you should change globals if you are not > intending to use them. If some value is needed which equals to > EG(scope) or other value depending on context, I think you should do > it with variable, not change the global, if possible. EG(scope) is used by zend_u_fetch_class to get the parent/current class, I'm only altering it temporarily before zend_u_fetch_class and restore it after that, no harm done. -- Etienne Kneuss http://www.colder.ch colder@php.net Men never do evil so completely and cheerfully as when they do it from a religious conviction. -- Pascal --- ## #29961 — Stanislav Malyshev — 2007-05-30T18:14:22+00:00 URL: https://externals.io/message/29953#29961 · In reply to: #29960 > EG(scope) is used by zend_u_fetch_class to get the parent/current class, > I'm only altering it temporarily before zend_u_fetch_class and restore > it after that, no harm done. I was speaking about 5.2 patch, didn't check the HEAD one. -- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/ --- ## #29967 — Etienne Kneuss — 2007-05-30T20:49:50+00:00 URL: https://externals.io/message/29953#29967 · In reply to: #29961 Stanislav Malyshev wrote: > > […] > > I was speaking about 5.2 patch, didn't check the HEAD one. Oh, I see now what you mean. I ported the patch to 5_2 with the assumption that zend_lookup_class also used EG(scope), but in fact it doesn't. I updated the patch[1] which now directly uses calling_scope/ce_org instead of the current scope for parent/self resolution. I hope this one better fits your requirements. Regards 1: http://patches.colder.ch/Zend/callbacks_5_2.patch?markup -- Etienne Kneuss http://www.colder.ch colder@php.net Men never do evil so completely and cheerfully as when they do it from a religious conviction. -- Pascal --- ## #30002 — Stanislav Malyshev — 2007-05-31T18:06:31+00:00 URL: https://externals.io/message/29953#30002 · In reply to: #29960 > It's already supported, but buggy right now. I don't believe fixing it > for odd cases will bother anyone. Well, I'm for one not sure that callback in form array('B', 'parent::foo') shouldn't error out instead (I know it works now, but it never states in the docs it's legal and I don't think it was intended to work). Otherwise next thing would be array('B', 'C::foo') and that'd be a real mess. -- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/ --- ## #30006 — Richard Lynch — 2007-05-31T20:33:54+00:00 URL: https://externals.io/message/29953#30006 · In reply to: #30002 > > […] > > Well, I'm for one not sure that callback in form array('B', > 'parent::foo') shouldn't error out instead (I know it works now, but > it > never states in the docs it's legal and I don't think it was intended > to > work). Otherwise next thing would be array('B', 'C::foo') and that'd > be > a real mess. Seems to me that anybody who thinks they need this can lookup the parent of B and construct the right array in the first place... I'm sure the patch is nifty, but it seems better to me to just error out as well. What about array('B', 'self::foo') -- I can see how that's needed to call the static method, I guess... -- Some people have a "gift" link here. Know what I want? I want you to buy a CD from some indie artist. http://cdbaby.com/browse/from/lynch Yeah, I get a buck. So? --- ## #30010 — Stanislav Malyshev — 2007-05-31T20:44:25+00:00 URL: https://externals.io/message/29953#30010 · In reply to: #30006 > What about array('B', 'self::foo') -- I can see how that's needed to > call the static method, I guess... But that won't be different from just array('B', 'foo') so you don't need self::... -- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/ --- ## #30013 — Richard Lynch — 2007-05-31T20:55:58+00:00 URL: https://externals.io/message/29953#30013 · In reply to: #30010 > > […] > > But that won't be different from just array('B', 'foo') so you don't > need self::... Duh. Sorry. I somehow had some imaginary instance of 'B' that was involved... Got lost in the clouds thinking about who needs anything like this at all. -- Some people have a "gift" link here. Know what I want? I want you to buy a CD from some indie artist. http://cdbaby.com/browse/from/lynch Yeah, I get a buck. So? --- ## #30018 — Marcus Boerger — 2007-05-31T22:51:46+00:00 URL: https://externals.io/message/29953#30018 · In reply to: #30006 Hello Richard, the feature actually exists because we need array($obj, 'parent::func') best regards marcus Thursday, May 31, 2007, 10:33:54 PM, you wrote: > > > […] > Seems to me that anybody who thinks they need this can lookup the > parent of B and construct the right array in the first place... > I'm sure the patch is nifty, but it seems better to me to just error > out as well. > What about array('B', 'self::foo') -- I can see how that's needed to > call the static method, I guess... > -- > Some people have a "gift" link here. > Know what I want? > I want you to buy a CD from some indie artist. > http://cdbaby.com/browse/from/lynch > Yeah, I get a buck. So? Best regards, Marcus --- ## #30021 — Stanislav Malyshev — 2007-05-31T23:09:09+00:00 URL: https://externals.io/message/29953#30021 · In reply to: #30018 > the feature actually exists because we need array($obj, 'parent::func') Why/when do you need that? -- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/ --- ## #30023 — Michael Wallner — 2007-06-01T06:11:12+00:00 URL: https://externals.io/message/29953#30023 · In reply to: #30021 Stanislav Malyshev wrote: > > […] > > Why/when do you need that? You'd probably do something along those lines if it were possible: ((ParentClass) $child)->virtualMethod(); If you prefer, s/You/One/ Regards, -- Michael --- ## #30031 — Stanislav Malyshev — 2007-06-01T17:06:32+00:00 URL: https://externals.io/message/29953#30031 · In reply to: #30023 > You'd probably do something along those lines if it were possible: > > ((ParentClass) $child)->virtualMethod(); Looks like bad style to me - why not call child's method and it would, if needed, pass control to parent? -- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/ --- ## #30033 — Richard Lynch — 2007-06-01T22:41:49+00:00 URL: https://externals.io/message/29953#30033 · In reply to: #30031 > > […] > > Looks like bad style to me - why not call child's method and it would, > if needed, pass control to parent? Or, if, for some reason, you need to bypass the child's method, and call only the parent's method of the same name, why not just look up the class->Parent and then use array(Parent, 'function') -- Some people have a "gift" link here. Know what I want? I want you to buy a CD from some indie artist. http://cdbaby.com/browse/from/lynch Yeah, I get a buck. So? --- ## #30038 — Jochem Maas — 2007-06-02T08:44:32+00:00 URL: https://externals.io/message/29953#30038 · In reply to: #30031 Stanislav Malyshev wrote: > > […] > > Looks like bad style to me - why not call child's method and it would, > if needed, pass control to parent? yeah, although I could imagine a situation where your stuck needing to calling parent::method() it smells like bad OO - the same kind of bad smell that people have been told they should avoid when it comes to wanting static interface methods, function signature changing, etc. personally I don't mind the smells, I'm not from the straight-jacket OO side of the street. that said I do feel consistency is a worthy goal and therefore it seems correct to enforce straight-jackets all round - given the general direction/mindset of php's OO functionality it seems that calling parent::method() from outside the relevant $child->method() should be illegal inline with other 'strictness enforcements'.