Prevent offsetGet on enums in const expr

[iluuu1994]

Based on this discussion. We need to prevent Foo::Bar[0] in const expressions because that would allow you to run arbitrary code in a constant expression (through offsetGet). This should probably be mentioned in the RFC.

Here's the tests, maybe that will make more sense:
iluuu1994/php-src@4f4c314

[iluuu1994]

/cc @TysonAndre

[TysonAndre]

It'd make more sense as a thrown Error (not a CompileError) because constants are lazily evaluated the first time a constant expression for a class is read from - see comment on linked commit

[Crell]

@iluuu1994 The "this discussion" link links back to this page. Where did you intend to have it point?

[Crell]

I'd be fine just disallowing ArrayAccess entirely. Otherwise it gets confusing as I don't quite grok from the test what is and isn't allowed with ArrayAccess.

That said, every method on an enum has the potential to bypass the singleton-ness of it. offsetGet() isn't special in that regard, is it? (offsetSet() is where it is definitely a no-no.)

[TysonAndre]

It was my earlier quote from the PR link - obviously any method can have state through statics and global variables, etc., but the issue is the change to constant expressions

The existence of https://www.php.net/arrayaccess may introduce some edge cases to handle for constant expression evaluation.

It'd be inconvenient to start allowing constants that aren't actually constant

The most straightforward solution would be to forbid implementing ArrayAccess entirely, similar to how
many magic methods are forbidden by this RFC (I'd recommend that). It may also be possible to allow it but check for mutable objects, references, or cyclical data structures

If something is already forbidding doing this, it'd be useful to add a test that the error is reported properly or the code behaves as expected.

EDIT: property defaults or parameter defaults may or may not break assumptions of the php vm, opcache, etc.

<?php
enum Foo implements ArrayAccess {
    case Bar;

    public function offsetExists($key) {}
    public function offsetGet($key) {
        return new stdClass();
    }
    public function offsetSet($key, $value) {}
    public function offsetUnset($key) {}
}
class X {
    const ACTUALLY_MUTABLE_OBJECT = Foo::BAR[0];
}
class Y {
    public stdClass $x = Foo::BAR[0];
}
var_dump(new Y());
function example(stdClass $default = Foo::BAR[0]) { var_dump($default); }
example();

[iluuu1994]

@TysonAndre

It'd make more sense as a thrown Error (not a CompileError) because constants are lazily evaluated the first time a constant expression for a class is read from - see comment on linked commit

AFAIK there are various things that can trigger the updating of the class constants (e.g. create a new instance). So the class will be completely unusable because attempting to do anything with it will always make it fail. Thus I'm not sure if recoverable behavior here is actually useful. WDYT?

[Crell]

Please clarify exactly what I should note is forbidden, because I don't really follow it. I'm fine blocking whatever you say to block here; I just need to grok it enough to document it.

[TysonAndre]

It's forbidden to use enum objects (from global constants, class constants, or enum values) in constant expressions

Please clarify exactly what I should note is forbidden, because I don't really follow it. I'm fine blocking whatever you say to block here; I just need to grok it enough to document it.

e.g. all of those would throw Errors at runtime

static $x = A::EnumValue[0];
static $x = ALIAS[0]; // for const ALIAS = A::EnumValue;
function example($x = A::EnumValue[0]) {}
const X = A::EnumValue[0];
class X {
    public $x = A::EnumValue[0];
    const X = A::EnumValue[0];
}

But something like $x = A::EnumValue[0]; would not throw at runtime, because that location isn't a location requiring a constant expression.

The concern is allowing a way to introduce side effects or non-deterministic behavior when evaluating constant expressions for enums that implement ArrayAccess - an unrealistic example is below:

<?php
class MyEvaluator implements ArrayAccess {
    public function offsetGet($offset) {
        return eval("return ($offset);");
    }
    public function offsetExists($offset) {
        return true;
    }
    public function offsetUnset($offset) {
    }
    public function offsetSet($offset, $value) {
    }
}
$e = new MyEvaluator();
var_dump($e['1+1']);
var_dump($e['rand(0,999)']);
var_dump($e['print("test\n")']);
/*
int(2)
int(508)
test
int(1)
 */

[Crell]

Hm. So we currently say this:

Additionally, because cases are represented as constants on the enum itself, they may be used as static values in constant definition or function parameter default values.

You're saying we should change it to, I think:

Additionally, because cases are represented as constants on the enum itself, they may be used as static values in constant definition or function parameter default values. However, ArrayAccess or method calls on enums are not allowed in static or constant definitions as we cannot absolutely guarantee that the resulting value is static.

Am I following?

[TysonAndre]

Additionally, because cases are represented as constants on the enum itself, they may be used as static values in constant expressions (property defaults, static variable defaults, parameter defaults, global and class constant values, or enum case values). However, implicit magic method calls such as ArrayAccess on enums are not allowed in static or constant definitions as we cannot absolutely guarantee that the resulting value is deterministic or that the method invocation is free of side effects. Function calls, method calls, and property access continue to be invalid operations in constant expressions.

I'd write it more like the above, but there may be a clearer way to put that.

Explicit method calls and property access syntaxes were never allowed in constant expressions in any php version, so the phrasing is confusing, e.g. in php 7.4 .

EDIT: The error message changed in php 8.0

php > const A = A->C;

Fatal error: Constant expression contains invalid operations in php shell code on line 1
php > const A = A->C();

Fatal error: Constant expression contains invalid operations in php shell code on line 1
php > const X = Y();

Fatal error: Constant expression contains invalid operations in php shell code on line 1

The only concern is implicit method calls such as ArrayAccess, and __toString(). Which reminds me, should invoking __toString() on enum classes in constant expressions also be forbidden? It must either return a string or throw, and people generally write deterministic __toString functions, so I don't have strong opinions about this. I haven't checked if ct_eval_const_expr allows this

(for const X = "" . Suit::Hearts;)

php > const X = (string)2;

Fatal error: Constant expression contains invalid operations in php shell code on line 1
php > const X = '' . 2;  // this is allowed
php > php > var_export(X);
'2'
php > class StringSideEffect { public function __toString() { static $i = 0; $i++; return "i=$i"; } }
php > $x = new StringSideEffect();
php > echo ('' . $x);
i=1
php > echo ('' . $x);
i=2

[Crell]

Ah, gotcha. __toString is already disallowed according to the RFC at the moment, so no worries there.

I've updated the RFC accordingly. I broke that description out to a new section rather than tucking it into the constants section. See the new "Enums in constant expressions" section and let me know if I captured it accurately.

[iluuu1994]

Explanation in the RFC seems very reasonable. 👍