Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] Unexpected results from constant folding

Posted by Sara Golemon 
Sara Golemon
[PHP-DEV] Unexpected results from constant folding
March 15, 2017 03:50AM
This comes in thanks to my old friend Fred Emmott on the HHVM project:
https://3v4l.org/vUHq3

class Foo {
const A = 1 << 0;
const B = self::A | self::C;
const C = 1 << 1;

}

class Bar extends Foo {
const A = 1 << 2;
const C = 1 << 3;
}

var_dump(decbin(Bar::B));
// HHVM result: 11
// PHP5 result: 1100
// PHP7 result: 1001

HHVM's result is clearly correct as `self::` refers to the defining
class and so should bind to Foo's values for A and C.
PHP5's result is at least rationally viable, although it effectively
redefines `self::` to the semantics of `static::` just for this case.
PHP7's result is... well, I can imagine how it occurs, but it can't be
called correct by any measure.

Opinions on the right thing to do here?
a) Leave it alone because it's been that way since 7.0
b) Revert to php5 behavior
c) Match HHVM's behavior

I vote C because that's the semantic meaning of self:: and we should
respect PHP's own language rules.
Barring that, I'd be okay with B as it's at least explainable without
too much mental gymnastics.
I straight up veto A. That's just cray-cray.

-Sara

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sherif Ramadan
Re: [PHP-DEV] Unexpected results from constant folding
March 15, 2017 04:20AM
I would definitely vote C as well. PHP 7's behavior makes even less sense
than PHP 5 in this case.

On Tue, Mar 14, 2017 at 10:43 PM, Sara Golemon <[email protected]> wrote:

> This comes in thanks to my old friend Fred Emmott on the HHVM project:
> https://3v4l.org/vUHq3
>
> class Foo {
> const A = 1 << 0;
> const B = self::A | self::C;
> const C = 1 << 1;
>
> }
>
> class Bar extends Foo {
> const A = 1 << 2;
> const C = 1 << 3;
> }
>
> var_dump(decbin(Bar::B));
> // HHVM result: 11
> // PHP5 result: 1100
> // PHP7 result: 1001
>
> HHVM's result is clearly correct as `self::` refers to the defining
> class and so should bind to Foo's values for A and C.
> PHP5's result is at least rationally viable, although it effectively
> redefines `self::` to the semantics of `static::` just for this case.
> PHP7's result is... well, I can imagine how it occurs, but it can't be
> called correct by any measure.
>
> Opinions on the right thing to do here?
> a) Leave it alone because it's been that way since 7.0
> b) Revert to php5 behavior
> c) Match HHVM's behavior
>
> I vote C because that's the semantic meaning of self:: and we should
> respect PHP's own language rules.
> Barring that, I'd be okay with B as it's at least explainable without
> too much mental gymnastics.
> I straight up veto A. That's just cray-cray.
>
> -Sara
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
Nikita Popov
Re: [PHP-DEV] Unexpected results from constant folding
March 15, 2017 11:40AM
On Wed, Mar 15, 2017 at 3:43 AM, Sara Golemon <[email protected]> wrote:

> This comes in thanks to my old friend Fred Emmott on the HHVM project:
> https://3v4l.org/vUHq3
>
> class Foo {
> const A = 1 << 0;
> const B = self::A | self::C;
> const C = 1 << 1;
>
> }
>
> class Bar extends Foo {
> const A = 1 << 2;
> const C = 1 << 3;
> }
>
> var_dump(decbin(Bar::B));
> // HHVM result: 11
> // PHP5 result: 1100
> // PHP7 result: 1001
>
> HHVM's result is clearly correct as `self::` refers to the defining
> class and so should bind to Foo's values for A and C.
> PHP5's result is at least rationally viable, although it effectively
> redefines `self::` to the semantics of `static::` just for this case.
> PHP7's result is... well, I can imagine how it occurs, but it can't be
> called correct by any measure.
>
> Opinions on the right thing to do here?
> a) Leave it alone because it's been that way since 7.0
> b) Revert to php5 behavior
> c) Match HHVM's behavior
>
> I vote C because that's the semantic meaning of self:: and we should
> respect PHP's own language rules.
> Barring that, I'd be okay with B as it's at least explainable without
> too much mental gymnastics.
> I straight up veto A. That's just cray-cray.
>

Yes, this should behave as C. See also https://bugs.php.net/bug.php?id=69676
for an existing bug report on the topic.

I think in 7.1 it might be even fairly simple so fix this: IIRC we now
store the defining CE on inherited constants, so we should know the scope
to resolve against.

Nikita
David Rodrigues
Re: [PHP-DEV] Unexpected results from constant folding
March 15, 2017 01:30PM
Just to make simpler to visualize this issue: https://3v4l.org/ZhYlm

```php
class Foo {
const A = 'Foo::A';
const B = self::A . ' and ' . self::C;
const C = 'Foo::C';

}

class Bar extends Foo {
const A = 'Bar::A';
const C = 'Bar::C';
}

var_dump(Bar::B);
```

You should note that:

PHP 7.0 to 7.1.2 => Foo::A and Bar::C
> It will give a result like a self::A and static::C (currently last one is
impossible);

PHP 5.6 to 5.6.30 => Bar::A and Bar::C
> It will give a result like a static::A and static::C;

hhvm 3.12.14 to 3.18.1 => Foo::A and Foo::B
> It will give a result like should be expected on code (both from self::
that is the Foo class);


2017-03-15 7:36 GMT-03:00 Nikita Popov <[email protected]>:

> On Wed, Mar 15, 2017 at 3:43 AM, Sara Golemon <[email protected]> wrote:
>
> > This comes in thanks to my old friend Fred Emmott on the HHVM project:
> > https://3v4l.org/vUHq3
> >
> > class Foo {
> > const A = 1 << 0;
> > const B = self::A | self::C;
> > const C = 1 << 1;
> >
> > }
> >
> > class Bar extends Foo {
> > const A = 1 << 2;
> > const C = 1 << 3;
> > }
> >
> > var_dump(decbin(Bar::B));
> > // HHVM result: 11
> > // PHP5 result: 1100
> > // PHP7 result: 1001
> >
> > HHVM's result is clearly correct as `self::` refers to the defining
> > class and so should bind to Foo's values for A and C.
> > PHP5's result is at least rationally viable, although it effectively
> > redefines `self::` to the semantics of `static::` just for this case.
> > PHP7's result is... well, I can imagine how it occurs, but it can't be
> > called correct by any measure.
> >
> > Opinions on the right thing to do here?
> > a) Leave it alone because it's been that way since 7.0
> > b) Revert to php5 behavior
> > c) Match HHVM's behavior
> >
> > I vote C because that's the semantic meaning of self:: and we should
> > respect PHP's own language rules.
> > Barring that, I'd be okay with B as it's at least explainable without
> > too much mental gymnastics.
> > I straight up veto A. That's just cray-cray.
> >
>
> Yes, this should behave as C. See also https://bugs.php.net/bug.php?
> id=69676
> for an existing bug report on the topic.
>
> I think in 7.1 it might be even fairly simple so fix this: IIRC we now
> store the defining CE on inherited constants, so we should know the scope
> to resolve against.
>
> Nikita
>



--
David Rodrigues
Sara Golemon
Re: [PHP-DEV] Unexpected results from constant folding
March 15, 2017 04:00PM
On Wed, Mar 15, 2017 at 5:36 AM, Nikita Popov <[email protected]> wrote:
> Yes, this should behave as C. See also https://bugs.php.net/bug.php?id=69676
> for an existing bug report on the topic.
>
Given that bug is assigned to you, I'll just leave it in your hands then?

> I think in 7.1 it might be even fairly simple so fix this: IIRC we now store
> the defining CE on inherited constants, so we should know the scope to
> resolve against.
>
Even in 7.0 you could rewrite unresolved self:: references to the
current classname.Not as clean when it comes to producing error
messages, but certainly a quick and easy fix for the case where the
userspace code isn't broken.

I had initially been thinking multi-pass, but there are some pretty
easy edge cases to fall into there...

-Sara

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Nikita Popov
Re: [PHP-DEV] Unexpected results from constant folding
March 15, 2017 10:20PM
On Wed, Mar 15, 2017 at 3:53 PM, Sara Golemon <[email protected]> wrote:

> On Wed, Mar 15, 2017 at 5:36 AM, Nikita Popov <[email protected]>
> wrote:
> > Yes, this should behave as C. See also https://bugs.php.net/bug.php?
> id=69676
> > for an existing bug report on the topic.
> >
> Given that bug is assigned to you, I'll just leave it in your hands then?
>
> > I think in 7.1 it might be even fairly simple so fix this: IIRC we now
> store
> > the defining CE on inherited constants, so we should know the scope to
> > resolve against.
> >
> Even in 7.0 you could rewrite unresolved self:: references to the
> current classname.Not as clean when it comes to producing error
> messages, but certainly a quick and easy fix for the case where the
> userspace code isn't broken.
>
> I had initially been thinking multi-pass, but there are some pretty
> easy edge cases to fall into there...


Fixed in 7.1 by
https://github.com/php/php-src/commit/2bba4a0d7f6d5e5712d60bc1cf2119622d837e55
..

I personally don't consider a PHP 7.0 backport worthwhile, as this is a
long standing issue (from early PHP 5 days) and the fix will probably be
hacky on 7.0. Note that CT resolving self:: doesn't quite cut it, because
you also have to deal with parent::, which the compiler currently does not
track. An alternative would be to force a full constant update on any
parent CE, to ensure they will be evaluated in the correct order. Of
course, this has other side effects, because it changes the point in time
at which a certain constant is updated.

Regards,
Nikita
Sara Golemon
Re: [PHP-DEV] Unexpected results from constant folding
March 15, 2017 10:40PM
On Wed, Mar 15, 2017 at 4:14 PM, Nikita Popov <[email protected]> wrote:
> Fixed in 7.1 by
> https://github.com/php/php-src/commit/2bba4a0d7f6d5e5712d60bc1cf2119622d837e55.
>
Thanks!

> I personally don't consider a PHP 7.0 backport worthwhile, as this is a long
> standing issue (from early PHP 5 days) and the fix will probably be hacky on
> 7.0.
>
Probably not, but I like to think out loud. :)

> Note that CT resolving self:: doesn't quite cut it, because you also
> have to deal with parent::, which the compiler currently does not track.
>
It doesn't track the specific ce (because it may not know the real
parent until runtime), but we have the extends node which gives us the
parent's name and that could be rewritten in the same way as self.

> An alternative would be to force a full constant update on any parent CE, to
> ensure they will be evaluated in the correct order. Of course, this has
> other side effects, because it changes the point in time at which a certain
> constant is updated.
>
Yeah, there's too many edge cases to go down that rabbit hole, and as
you say, it's a much older bug than 7.0 even. So best face forwards
on this one.

-Sara

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login