Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:103618
MIME-Version: 1.0
References: <CAFMT4Nodo_8dw4UkwZqT4vzE1=27xd7wczf=JxYu=R3j71xXag@mail.gmail.com>
 <DM6PR02MB593219A576A1618E7D600C9DBFBF0@DM6PR02MB5932.namprd02.prod.outlook.com>
 <DM6PR02MB5932213FE22E684434FD1557BFBF0@DM6PR02MB5932.namprd02.prod.outlook.com>
In-Reply-To: <DM6PR02MB5932213FE22E684434FD1557BFBF0@DM6PR02MB5932.namprd02.prod.outlook.com>
Date: Thu, 20 Dec 2018 17:12:52 -0700
Message-ID: <CAFMT4NrRQnkttJDdBXpyB=8EQC62_MnRfGAJU1hPp3nbUCW-dA@mail.gmail.com>
To: Dmitry Stogov <dmitry@zend.com>
Cc: internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] [RFC][Vote] Covariant Returns and Contravariant Parameters
From: levim@php.net (Levi Morrison)

On Thu, Dec 20, 2018 at 3:35 PM Dmitry Stogov <dmitry@zend.com> wrote:
>
> Hi Levi,
>
>
> It looks like the patch broke something related to opcache.
>
> It crashes at least on Wordpress and Drupal.
>
> The backtrace https://gist.github.com/dstogov/a2305381a5c9982cceca9e4e252d26c7 shows use-after-free in opcache (works fine with master).
>
> Inability to work with opcache, doesn't allow to check the performance impact.
>
>
> It also broke few tests. Some crash. Some produce different warning/errors.
>
>
> $ make test TESTS="Zend tests"
>
> ...
>
> =====================================================================
> FAILED TEST SUMMARY
> ---------------------------------------------------------------------
> ZE2 ArrayAccess and ArrayAccessReferenceProxy with references to main array [tests/classes/array_access_011.phpt]
> Bug #21478 (Zend/zend_alloc.c :: shutdown_memory_manager produces segfault) [Zend/tests/bug21478.phpt]
> Generator methods can yield by reference [Zend/tests/generators/generator_method_by_ref.phpt]
> Testing to implement Serializable interface by traits [Zend/tests/traits/interface_003.phpt]
> Handling of public fields with traits needs to have same semantics as with normal inheritance, however, we do add strict warnings since it is easier to run into something unexpeted with changing traits. [Zend/tests/traits/property009.phpt]
> iterable type#004 - Parameter covariance [Zend/tests/type_declarations/iterable_004.phpt]
> iterable type#005 - Return type covariance [Zend/tests/type_declarations/iterable_005.phpt]
> =====================================================================
>
> I'll try to play with patch and make a full code review on next week.
>
>
> It would be great, if you fix opcache compatibility.
>
> If it can't be done in reasonable time, it's probably better to cancel voting and restart when ready.

What OS and compiler are these on? How are you ensuring that opcache
is on when these tests are run? I have not been experiencing these
issues, so maybe I am not running it correctly. If I cannot reproduce
them soon then I will agree to cancel the voting.

There are some known issues outside of Zend. Notably some internal
classes do not have valid method signatures with regards to
inheritance which this patch exposed. These need fixed regardless of
this RFC and I have begun work on some of them (see
https://github.com/php/php-src/pull/3686 for one example).