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
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]
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
... 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
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