Skip to content
This repository has been archived by the owner on Jun 10, 2019. It is now read-only.

PHP7 incompatibility - several uses of &new in /lib #8

Closed
durzel opened this issue Dec 18, 2015 · 4 comments
Closed

PHP7 incompatibility - several uses of &new in /lib #8

durzel opened this issue Dec 18, 2015 · 4 comments

Comments

@durzel
Copy link
Contributor

durzel commented Dec 18, 2015

Hi,

When trying to compile all of the PHP files under Magento using OpCache I ran into some issues, namely that there are several places under /lib where the result of a new statement is assigned to a variable by reference. This was depreciated in PHP 5.6 and is now a parsing error in 7.x, and blocks compilation.

http://php.net/manual/en/migration70.incompatible.php#migration70.incompatible.other.new-by-ref

Affected files (Magento CE 1.9.2.2):

lib/PEAR/XML/Unserializer.php:260: * $unserializer = &new XML_Unserializer();
lib/PEAR/XML/Unserializer.php:801:                $value['value'] = &new $classname;
lib/PEAR/XML/Unserializer.php:974:        $this->_parser = &new XML_Parser($this->
lib/PEAR/XML/Parser/Simple.php:82: * $p = &new myParser();
lib/PEAR/XML/Parser.php:616:        $err = &new XML_Parser_Error($msg, $ecode);
lib/PEAR/HTTP/Request.php:110: * $a = &new HTTP_Request('http://www.yahoo.com/');
lib/Varien/Pear/Registry.php:49:                    $this->_config = &new PEAR_Config($this->statedir . DIRECTORY_SEPARATOR .

There is nothing under /app that is uses &new - so that's safe.

I was going to submit a pull request for this, but since it affects core files that can't be extended (without some trickery) I thought best to simply post this issue so people will be aware.

@durzel
Copy link
Contributor Author

durzel commented Dec 18, 2015

Just noticed technically 3 of those are in comments, so lib/PEAR/XML/Parser/Simple.php and lib/PEAR/HTTP/Request.php are ok.

@icurdinj
Copy link
Contributor

Registry.php can probably be ignored, since the problematic code is in a code path commented with "never used". :-)
In Parser.php, problematic code is in raiseError function. Again, probably not very important, because it will error when it errors. :-)
Unserializer.php worries me, though. On first look, I can't find usages of 2 problematic functions here. Can anyone produce an error here, in any situation other than compiling all?

@durzel
Copy link
Contributor Author

durzel commented Feb 4, 2016

Sorry forgot to reply to this. Not encountered any issue in practice with this code, but it does block compilation of all PHP files which is not the end of the world, but can be a bit annoying if you want everything compiled.

@icurdinj
Copy link
Contributor

I had time to play with this now - it seems this code can be trivially fixed by replacing &new with new. (Both work the same since PHP 5.0). But, it also seems that this code is practically never used in Magento...
So, no point in doing overwrites in this extension, but if we go ahead and submit a core patch, we'll fix this too.
Workaround if you want to compile all in opcache - put problematic files in opcache blacklist (see opcache.blacklist_filename setting).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants