All,
Currently, use has a few error cases that are rather fragile. For example:
namespace Biz {
class Bar {};
}
namespace Foo {
use Biz\Bar;
$a = new Bar;
var_dump($a);
}
namespace Foo {
class Bar {};
}
http://3v4l.org/IPTiQ
That works 100% of the time, on 5.3.0 -> present.
But if we change the order of the two Foo blocks:
namespace Biz {
class Bar {};
}
namespace Foo {
class Bar {};
}
namespace Foo {
use Biz\Bar;
$a = new Bar;
var_dump($a);
}
http://3v4l.org/KlXUq
We get a Fatal error that "Cannot use Biz\Bar as Bar because the name
is already in use.
Basically, zend_compile.c is doing a check of the current symbol table
to see if the class is already defined when doing the use.
This is problematic not just for the example above, but when combined
with opcache. Opcache nulls out the symbol tables for file
compilation. So that means that file inclusion order matters when it's
disabled, but not when it's enabled.
This was discovered last night by Drupal 8 developers, when they
noticed that some developers claimed fatal errors, while they weren't
able to reproduce. After some debugging, it turned out that disabling
opcache caused the fatal errors. It was because of this symbol table
change.
The error is fragile. And IMHO it's not really protecting anything
significant, since it's clear within the file what each symbol refers
to (you need to look at use declarations for that anyway).
So, I created a PR to remove this error:
https://github.com/php/php-src/pull/1149
Note that there is no BC break here, as it's removing an error condition today.
This results in a weird edge case (which is 100% valid, but feels odd):
<?php
class Test {}
use Bar\Test;
var_dump(new Test); // class(Bar\Test)
That's perfectly valid, if not a bit weird.
The reverse would error mind you:
<?php
use Bar\Test;
class Test {}
Because you've already defined the symbol Test in the file.
To get around this, I've created another PR with a BC break, adding a
compile error if code comes before `use`:
https://github.com/php/php-src/pull/1150
This requires use to immediately follow namespace declarations:
namespace Foo {
use Bar; //valid
}
namespace Bar {
use Foo; //valid, second namespace in file
}
namespace Baz {
echo "Hi!";
use Foo; // Invalid
}
This did break approximately 30 internal tests, but they were all
testing this specific code path (or bugs related to it).
What are your thoughts about both PRs? The first seems like a clear
win, the second is a nice cleanup but also a bit of a change to
behavior... I'm not sure if an RFC is required for either, but if
people want one (for both, for one, etc) I can draft something up
quickly.
Remove symbol check: https://github.com/php/php-src/pull/1149
Pin use to top: https://github.com/php/php-src/pull/1150
Thanks
Anthony
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
