Hi internals,
Currently
class Foo {}
new Foo(print 'xyz');
will not print "xyz", because the arguments to "new" are not evaluated if
the class has no constructor. Conversely
class Foo { function __construct() {} }
new Foo(print 'xyz');
will print "xyz". This behavior is confusing, especially if the code is
new $class(...) where $class may or may not have a constructor and thus the
side effects of "..." may or may not occur, leading to a hard to identify
failure mode.
I do not believe this behavior is intentional, it is simply an artifact of
the precise way in which "new" was implemented. HHVM does not implement
"new" in this way, they always evaluate the arguments.
We regularly get bug reports about this behavior. A sample of recent ones I
was able to track down: #54162, #54170, #65930, #67829 and #70698.
I'd like to fix this behavior, so that new arguments are always evaluated.
Here's a patch: https://github.com/php/php-src/pull/1802
As this is technically a BC affecting change (even if of the lowest
impact), I'm running it past the list first.
Thanks,
Nikita
class Foo {} new Foo(print 'xyz');will not print "xyz", because the arguments to "new" are not evaluated if
the class has no constructor. Conversely
I recall someone once dubbing this "The Rasmus Optimization". I don't
know if that means it was intentional, or just a lazy jibe, but yeah.
It's a long known issue, so it's probably good that you posted about
it first, as someone may have a legit reason for not wanting to fix
it. (I don't, I regard this as a php sadness)
HHVM does not implement "new" in this way, they always evaluate the arguments.
Which, if it helps, means that we already know a lot of frameworks
/don't/ break as a result of fixing this behavior.
As this is technically a BC affecting change (even if of the lowest
impact), I'm running it past the list first.
Technically a BC break that justifies waiting till a major version.
Given the extremely specific requirements to hit this edge case
however, passing side-effect args to a non-existent constructor, I
personally think a minor version is fine.
+1 (without having looked at the patch)
-Sara
HHVM does not implement "new" in this way, they always evaluate the arguments.
Which, if it helps, means that we already know a lot of frameworks
/don't/ break as a result of fixing this behavior.As this is technically a BC affecting change (even if of the lowest
impact), I'm running it past the list first.Technically a BC break that justifies waiting till a major version.
Given the extremely specific requirements to hit this edge case
however, passing side-effect args to a non-existent constructor, I
personally think a minor version is fine.
Of curiosity, what effect (if any) might this have on ReflectionClass::newInstance(), ::newInstanceArgs(), and ::newInstanceWithoutConstructor()? (I ask because some existing DI systems, such as Aura.Di, rely on those for creating objects.)
--
Paul M. Jones
pmjones88@gmail.com
http://paul-m-jones.com
Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp
Solving the N+1 Problem in PHP
https://leanpub.com/sn1php
HHVM does not implement "new" in this way, they always evaluate the
arguments.Which, if it helps, means that we already know a lot of frameworks
/don't/ break as a result of fixing this behavior.As this is technically a BC affecting change (even if of the lowest
impact), I'm running it past the list first.Technically a BC break that justifies waiting till a major version.
Given the extremely specific requirements to hit this edge case
however, passing side-effect args to a non-existent constructor, I
personally think a minor version is fine.Of curiosity, what effect (if any) might this have on
ReflectionClass::newInstance(), ::newInstanceArgs(), and
::newInstanceWithoutConstructor()? (I ask because some existing DI
systems, such as Aura.Di, rely on those for creating objects.)
There is no effect on any the ReflectionClass::newInstance*() methods, only
direct use of the "new" operator is affected.
Nikita