Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:84380
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.223.179 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAAyV7nERc=GWfP0GrUkjjvMHgLBjDno3_=0nw-y7JzLSZ3ZEHQ@mail.gmail.com>
References: <CAAyV7nERc=GWfP0GrUkjjvMHgLBjDno3_=0nw-y7JzLSZ3ZEHQ@mail.gmail.com>
Date: Fri, 6 Mar 2015 12:06:15 +0000
Message-ID: <CAHZ_-MBgZp+iQd0MpnNq0ToUuqF37X5c-jOVbVgJXu9fiSx6jg@mail.gmail.com>
To: Anthony Ferrara <ircmaxell@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=089e013a1118e1e1b505109d820d
Subject: Re: [PHP-DEV] Use behavior thoughts
From: leedavis81@gmail.com (Lee Davis)

--089e013a1118e1e1b505109d820d
Content-Type: text/plain; charset=UTF-8

Hi Anthony.

   This issue that has plagued me in the past, specifically with the use of
traits:

Error:  http://3v4l.org/VFguK
OK:     http://3v4l.org/73b86

Although when combined with opcache does causes very confusing behaviour, I
do worry that removing the error altogether may make it worse for
developers.

Take this example with constants:

namespace Biz {
    const Bar = 41;
}

namespace Foo {
    const Bar = 42;
}

namespace Foo {
    use const Biz\Bar;
    echo Bar;
}

// Fatal error: Cannot use const Biz\Bar as Bar because the name is already
in use

http://3v4l.org/0pRZk

An error here is far more useful that trying to exhibit the behaviour of
when the namespaces are declared in reverse order:

namespace Biz {
    const Bar = 41;
}

namespace Foo {
    use const Biz\Bar;
    echo Bar;
}

namespace Foo {
    const Bar = 42;
}

// 41

http://3v4l.org/E4PFX

I suppose the question here is; Does the declaration order matter?
Personally I think it should.


/@leedavis81



On Thu, Mar 5, 2015 at 7:40 PM, Anthony Ferrara <ircmaxell@gmail.com> wrote:

> 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
>
>

--089e013a1118e1e1b505109d820d--