Wired constant expression syntax and bug View as Markdown

Hi Bob,

I'm wondered why you introduced this wired syntax in PHP-5.6.

Do you mean weird?

yes :) sorry

class FooBar {
const bar = ["bar" => 3]["bar"];
}

It wasn't a part of RFC, it wasn't covered by tests, and it actually
doesn't make a lot of sense. May be it's better to remove it?

I disagree, it makes perfect sense:

class FooBar {
const FOO = 3;
const BAR = [
3 => ‘qux’,
4 => ‘bang’,
7 => ‘theta’,
9 => ‘epsilon’
][FOO];
}

?: and ? only work when there are just two possibilities.

?: may work with brackets, but I have to admit that it's less readable.

class Foo {
const X = 2;
const Y = (Foo::X == 0 ? 1 :
(Foo::X == 1 ? 2 :
(Foo::X == 2 ? 3 :
(Foo::X == 3 ? 4 :
5))));
}

The situation is really inconsistent because we allow expressions on
constant arrays but at the same time prohibit array usage.

As we support the [..., ...][...] syntax in normal PHP code as well, I think
it's reasonable to allow it here as well. Of course it isn't very
practically useful, but there doesn't seem much point to explicitly
disallowing it here.

Look into the following scripts:

<?php
class Foo {
const BAR = [
3 => ‘qux’,
4 => ‘bang’,
7 => ‘theta’,
9 => ‘epsilon’
][1];
}
var_dump(Foo::BAR); // works - prints 'bang'
?>

<?php
class Foo {
const BAR = [
3 => ‘qux’,
4 => ‘bang’,
7 => ‘theta’,
9 => ‘epsilon’
];
}
var_dump(Foo::BAR); // doesn't work - Fatal error: Arrays are not allowed
in constants at run-time
?>

<?php
class Foo {
const BAR = [
3 => ‘qux’,
4 => ‘bang’,
7 => ‘theta’,
9 => ‘epsilon’
];
}
// This works again! We may declare array class constants if we don't use
them?
?>

Where is the logic?

The reason here is probably that we cannot detect whether something will be
an array or not until runtime constant updating. E.g. const BAR = FOO ? 1 :
[1] or similar. However we might want to detect the case where an array is
the root of the AST, as that is invalid for sure and the most common case.

Another small issue with the constant expressions I noticed is that our
recursion detection doesn't properly work for constant ASTs. For example:

<?php
class A {
const FOO = [self::BAR];
const BAR = [self::FOO];
}
var_dump(A::FOO);

This will result in a stack overflow instead of a fatal error about
self-referencing constants.

So what's the status ?
We release RC2 this week. Final is not far, and as I can read, it's not stable.

So I suggest we use more RCs to try to make this stable (and I mean
with OPCache activated).
If we can't make it in reasonnable time, then the feature will have to
be reverted.

Please, note we won't have too many RCs as we are already late on our
original timetable.

Julien Pauli

Am 1.7.2014 um 09:12 schrieb Nikita Popov nikita.ppv@gmail.com:

class FooBar {
const bar = ["bar" => 3]["bar"];
}

It wasn't a part of RFC, it wasn't covered by tests, and it actually
doesn't make a lot of sense. May be it's better to remove it?

I disagree, it makes perfect sense:

class FooBar {
const FOO = 3;
const BAR = [
3 => ‘qux’,
4 => ‘bang’,
7 => ‘theta’,
9 => ‘epsilon’
][FOO];
}

?: and ? only work when there are just two possibilities.

?: may work with brackets, but I have to admit that it's less readable.

class Foo {
const X = 2;
const Y = (Foo::X == 0 ? 1 :
(Foo::X == 1 ? 2 :
(Foo::X == 2 ? 3 :
(Foo::X == 3 ? 4 :
5))));
}

The situation is really inconsistent because we allow expressions on
constant arrays but at the same time prohibit array usage.

As we support the [..., ...][...] syntax in normal PHP code as well, I think it's reasonable to allow it here as well. Of course it isn't very practically useful, but there doesn't seem much point to explicitly disallowing it here.

That's exactly my point.

Look into the following scripts:

<?php
class Foo {
const BAR = [
3 => ‘qux’,
4 => ‘bang’,
7 => ‘theta’,
9 => ‘epsilon’
][1];
}
var_dump(Foo::BAR); // works - prints 'bang'
?>

<?php
class Foo {
const BAR = [
3 => ‘qux’,
4 => ‘bang’,
7 => ‘theta’,
9 => ‘epsilon’
];
}
var_dump(Foo::BAR); // doesn't work - Fatal error: Arrays are not allowed
in constants at run-time
?>

<?php
class Foo {
const BAR = [
3 => ‘qux’,
4 => ‘bang’,
7 => ‘theta’,
9 => ‘epsilon’
];
}
// This works again! We may declare array class constants if we don't use
them?
?>

Where is the logic?

The reason here is probably that we cannot detect whether something will be an array or not until runtime constant updating. E.g. const BAR = FOO ? 1 : [1] or similar. However we might want to detect the case where an array is the root of the AST, as that is invalid for sure and the most common case.

Not sure if we should check that. That would be adding a new arbitrary restriction. Now the following works:

class Foo {
const BAR = [
3 => ‘qux’,
4 => ‘bang’,
7 => ‘theta’,
9 => ‘epsilon’
];

const BAZ = FOO[4];

}

var_dump(Foo::BAZ);

Another small issue with the constant expressions I noticed is that our recursion detection doesn't properly work for constant ASTs. For example:

<?php
class A {
const FOO = [self::BAR];
const BAR = [self::FOO];
}
var_dump(A::FOO);

This will result in a stack overflow instead of a fatal error about self-referencing constants.

Nikita

I think that this is caused by some bugfix for opcache.

http://lxr.php.net/xref/PHP_5_6/Zend/zend_ast.c#255
That zval_copy_ctor call duplicates the AST to save it for opcache so that opcache can store it later.
But as that is called before the zend_ast_evaluate, the effects of MARK_CONSTANT_VISITED() won't affect the contents of the ast (and the zval with the constant) copied before…

@Dmitry: Removing this call (and the dtoring in the ast destructor) will break opcache and fix this bug. How to fix both now? I don't immediately see a good fix here?

Bob

Hi Bob,

Now I think it's not fixable by design :(

I'll try to think about it later today.
Could you please collect all related issues.

Thanks. Dmitry.

Am 2.7.2014 um 15:43 schrieb Dmitry Stogov dmitry@zend.com:

I don't have good ideas out of the box and I probably won't be able to
look into this before next week.

Hey, I still have no real idea how to solve it without breaking opcache.

This one seems to be considered like a blocking bug for 5.6.

Could you please try to fix this in a sane manner?

Bob