Unexpected results from constant folding View as Markdown

Just to make simpler to visualize this issue: https://3v4l.org/ZhYlm

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 nikita.ppv@gmail.com:

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

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