Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:56372
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.161.170 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <F2.14.30628.CA275CE4@pb1.pair.com>
References: <F2.14.30628.CA275CE4@pb1.pair.com>
Date: Thu, 17 Nov 2011 22:11:26 +0100
Message-ID: <CAH-PCH7raacjA-96ny47KM6mw4L8+uXppomJGWaSKj_V6cZeLw@mail.gmail.com>
To: "Matthew Weier O'Phinney" <weierophinney@php.net>
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary=20cf3074af7cd2cddf04b1f4a9b5
Subject: Re: [PHP-DEV] Results of testing ZF against PHP 5.4.0RC1
From: tyra3l@gmail.com (Ferenc Kovacs)

--20cf3074af7cd2cddf04b1f4a9b5
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi Matthew, my replies are inline

On Thu, Nov 17, 2011 at 9:46 PM, Matthew Weier O'Phinney <
weierophinney@php.net> wrote:

> Greetings!
>
> My team and I (which means Ralph Schindler and Enrico Zimuel) took some
> time this week to:
>
>  * Build PHP 5.4.0RC1 and run make tests (and send feedback)
>  * Run Zend Framework 1.11 unit tests against PHP 5.4.0RC1
>  * Run Zend Framework 2.0 (dev) unit tests against PHP 5.4.0RC1
>
> We found the following issues when running the tests, and I've ordered
> them starting with those that affect us most. Overall, however, 5.4.0RC1
> largely worked out of the box.
>

nice


>
>  * Array to string conversion notices break tests
>   In 5.2 and 5.3, array to string conversions raise no notices, and as
>   such allow the tests to continue to run. Now, however, a notice is
>   raised, and PHPUnit then raises an error.
>
>   While I understand we should likely test values before using them in
>   string contexts, this feels like an arbitrary change at this time.
>
>   I recall from earlier discussions that Stas was in favor of reverting
>   this change -- what's the status at this time? This single change
>   will likely require a fair bit of time for us to fix across our ZF
>   versions if not reverted, and I suspect it will hit a lot of other
>   projects.
>

We didn't see one sane use-case where it is intended to compare an array as
'Array' to another string, so I think that albeit you aren't the only ones
bumping into this, but usually this means broken code/tests.
Chx also reported some drupal tests failing because of this change:
https://bugs.php.net/bug.php?id=3D60198 however as Rasmus explained that wa=
s
a bug on their part (they were expecting that array_diff/array_intersect
would correctly work with nested arrays), and it is sheer luck that they
are getting the "expected" results.


>  * ob_get_status(true) does not always return an array
>   One of our cache adapters uses ob_get_status(true) to determine
>   things like nesting levels. However, despite the fact that we're
>   passing the boolean true value to force return of an array, we get a
>   non-array when the output buffer is empty now -- and this is true
>   only of 5.4.0RC1. With the $full_status argument, I'd argue this
>   should _always_ return an array, even if that array is empty -- which
>   is precisely how it's documented. I see no reason for the change in
>   5.4.
>

Agree.


>  * ob_get_clean() now raises a notice if no buffer to delete
>   Prior to 5.4.0RC1, if there were no buffers left to delete,
>   ob_get_clean() was silent. It now raises a notice -- which, when
>   using PHPUnit, means an error is now raised, making the test fail.
>   I do not see any benefit to raising the notice; if there's nothing
>   left to clean, return an empty string or false (which was the former
>   behavior, and is documented).
>

Agree, the new behavior maybe a little bit more explicit, but I think it is
not intentional, just a side effect, but I can be wrong ofc.


>
>  * Redefining a constructor with different arguments now results in E_FAT=
AL
>   Prior to 5.4.0RC1, if a concrete class added arguments to a
>   constructor defined in an abstract class, no warning was raised. This
>   behavior now results in an E_FATAL. I'm ambivalent about this change,
>   however -- the code that raised this for us should be changed
>   anyways.
>
>   However, I'll argue, as others have on the list, that constructors
>   should have the flexibility of redefinition without raising notices
>   or compilation errors.
>

Yeah, this change is mentioned here:
https://bugs.php.net/bug.php?id=3D55085it was only a fix for
https://bugs.php.net/bug.php?id=3D51421
We have a lengthy discussion about this change as it is a little bit too
drastic: it prevents some sane use-cases, and triggering E_FATAL is also a
little bit too sever.
For the record here is a link for the thread on the mailing list:
http://www.mail-archive.com/internals@lists.php.net/msg53561.html
and Etienne Kneuss wrote an RFC on the current (as of 5.4) behavior which
could be used as a base to what should we change/refine:
https://wiki.php.net/rfc/prototype_checks
As I mentioned in the original thread I would like to see both the
restricted cases and the error level relaxed.


>
>  * Introduction of the new keyword "callable" means that this can no
>   longer be used for classnames, function names, or method names.
>
>   We had one case where a "callable()" method was defined, and this now
>   breaks; fortunately, it's only in tests, so we can easily fix it with
>   no BC issues on our part. I'll note that I've also used the
>   class|interface name "Callable" in the past on other projects.
>
>   I'm okay with this change, but it needs to be clearly spelled out in
>   the migration docs.
>

Agree.


--=20
Ferenc Kov=C3=A1cs
@Tyr43l - http://tyrael.hu

--20cf3074af7cd2cddf04b1f4a9b5--