return /* by reference */ new Foo() in PHP4

php.internals

Matthias Pigulla

20 years ago
Hi all, first of all I do not want to set off yet another discussion about the changes 4.4 brought. I do understand why the changes were necessary, and in most of the cases, I would even endorse that pieces of code that trigger the new "only variable..." waring are "bad code". Anyways, to me it seems that there are two very common patterns in OOP code that have problems with the new behaviours. The first one is returning-by-reference a reference obtained from another function. This was bug #33558 and has been fixed in 4.4.1RC1. The other one is as to "return new ...", which is quite common (think of factory methods!). Just as $x =& new Foo() needs to assign by ref to make sure $x and $this (inside the constructor) point to the very same instance, something like function &createInstance() { return new Foo(); } $x =& createInstance(); has to be done exectly like this with PHP4. Yes, I *do* understand that it is evaluated as an expression and thus generates a warning. The point is that this requires really unlogic and silly workarounds like 'return $tmp =& new Foo()'. That forces people to touch stable codebases; I find it comprehensible that they feel this is like passing the engine internal problems to the php coders. Even more disturbing, this does *not* generate a notice if inside Foo's constructor, $this is assigned by reference to something else. Say, in Foo's constructor, you have something like $someObservableObj->registerObserver($this), where registerObserver (of course) takes a reference and adds the observer to an array or something like that. Thus, it depends on implementation internals of Foo's constructor if "return new" - an implementation detail of a the factory! - produces the notice. The bug describing this is #33679, marked as bogus with no further explanation. So, in case of "return new", wouldn't it make sense to remove the warning as the code is 'legal'? And please, don't start a new "it's just a friendly notice" flame war. Best regards, Matthias

Oliver Grätz

20 years ago
Matthias Pigulla schrieb:
> So, in case of "return new", wouldn't it make sense to remove the > warning as the code is 'legal'? And please, don't start a new "it's just > a friendly notice" flame war.
Throwing a notice here is complete nonsense. All thinkable languages use this coding style of "return new". If I return a reference to something new and there's no name for it yet then a temporary internal name for it has to be created until the object is given a real name or it is no longer referenced. This is valid for all "only variables can be returned by reference" cases where the object originates from userspace.
> The bug describing this is #33679, marked as bogus with no further > explanation.
I had a similar experience with http://bugs.php.net/bug.php?id=34783 which tells that with objects implementing ArrayAccess usage of $object['index_using_the_interface']['subindex_for_index_that_is_array'] can't be made to work since 5.1. First of all, "won't fix" would have been the correct response instead of "bogus" since a very similar problem was marked "won't fix". But there are two things very problematic with this: - This worked perfectly in 5.0.3 (I didn't use 5.0.4 so I don't know how 5.0.4 reacts). - You mustn't create an interface that pretends to mimic arrays and then make it impossible to fully simulate array behaviour. The explanation for marking the bug bogus was "IT IS IMPOSSIBLE TO HAVE ARRAYACCESS DEAL WITH REFERENCES.". Then why did this work in 5.0.3? Okay, perhaps because of a more lax way of parsing the interface definition. But then this definition was false in the first place and should be corrected to support references. And if this is out of question then at least some workaround has to be implemented to support the sub-array-syntax as this has nothing to do with references from a user's point of view. Think a few years into the future with ArrayAccess implementations all over the place and people trying to use them as arrays. Because these implementations are hidden in frameworks people will totally freak out because sometimes $a[1][2] works and sometimes they get a totally incomprehensible "Objects used as arrays in post/pre increment/decrement must return values by reference" as fatal error when using such simple code. If they can't make the ArrayAccess to work like real arrays then it would be a better way to completely dump it (or lock it against implementation in userspace) instead of letting inconsistent behaviour creep into the language concepts. But, like with your bug, the bogusing bot is quite quick these days where just CLOSING mem leaks has priority over FIXING them from a PHP user's point of view. Perhaps the PHP coders should keep _all_ PHP code from being executed as this would certainly close every thinkable leak. It's not wise having users turn their back on key features of 5.x when the switch from 4.x to 5.x hasn't even really started. AllOLLi ____________ "Your DNA must cry itself to sleep at night." [Coupling]

Jani Taskinen

20 years ago
On Sat, 8 Oct 2005, Matthias Pigulla wrote:
> So, in case of "return new", wouldn't it make sense to remove the > warning as the code is 'legal'? And please, don't start a new "it's just > a friendly notice" flame war.
It's just a notice, tune your error_reporting level accordingly. --Jani

Todd Ruth

20 years ago
On Sat, 2005-10-08 at 21:57 +0200, Matthias Pigulla wrote:
> ... This > was bug #33558 and has been fixed in 4.4.1RC1.
Does that give 34551 any chance of being fixed? 33558 is just an annoying notice, but 34551 is a real BC break in 4.4 that wasn't documented and will likely continue to bite people who don't corrupt memory and think they can safely move to 4.4. - Todd

Derick Rethans

20 years ago
On Sat, 8 Oct 2005, Matthias Pigulla wrote:
> The point is that this requires really unlogic and silly workarounds > like 'return $tmp =& new Foo()'. That forces people to touch stable > codebases; I find it comprehensible that they feel this is like passing > the engine internal problems to the php coders.
I discussed this with Dmitry today and we agree that although this is strange, we will not do any modifications here as PHP 4.4. is stable and we don't want to risk breaking things. Derick
-- Derick Rethans http://derickrethans.nl | http://ez.no | http://xdebug.org