Skip to content

Commit ddc0b49

Browse files
committed
Allow arbitrary const expressions in backed enums
Closes GH-7821 Closes GH-8190 Closes GH-8418
1 parent 5a855ee commit ddc0b49

23 files changed

+287
-144
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ PHP NEWS
77
references). (Nicolas Grekas)
88
. Fixed bug GH-8661 (Nullsafe in coalesce triggers undefined variable
99
warning). (ilutov)
10+
. Fixed bug GH-7821 and GH-8418 (Allow arbitrary const expressions in backed
11+
enums). (ilutov)
1012

1113
- MBString:
1214
. Backwards-compatible mappings for 0x5C/0x7E in Shift-JIS are restored,

Zend/tests/enum/backed-duplicate-int.phpt

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,33 @@ enum Foo: int {
88
case Baz = 0;
99
}
1010

11+
try {
12+
var_dump(Foo::Bar);
13+
} catch (Error $e) {
14+
echo $e->getMessage(), "\n";
15+
}
16+
17+
try {
18+
var_dump(Foo::Bar);
19+
} catch (Error $e) {
20+
echo $e->getMessage(), "\n";
21+
}
22+
23+
try {
24+
var_dump(Foo::from(42));
25+
} catch (Error $e) {
26+
echo $e->getMessage(), "\n";
27+
}
28+
29+
try {
30+
var_dump(Foo::tryFrom('bar'));
31+
} catch (Error $e) {
32+
echo $e->getMessage(), "\n";
33+
}
34+
1135
?>
12-
--EXPECTF--
13-
Fatal error: Duplicate value in enum Foo for cases Bar and Baz in %s on line %s
36+
--EXPECT--
37+
Duplicate value in enum Foo for cases Bar and Baz
38+
Duplicate value in enum Foo for cases Bar and Baz
39+
Duplicate value in enum Foo for cases Bar and Baz
40+
Foo::tryFrom(): Argument #1 ($value) must be of type int, string given

Zend/tests/enum/backed-duplicate-string.phpt

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,33 @@ enum Suit: string {
1010
case Spades = 'H';
1111
}
1212

13+
try {
14+
var_dump(Suit::Hearts);
15+
} catch (Error $e) {
16+
echo $e->getMessage(), "\n";
17+
}
18+
19+
try {
20+
var_dump(Suit::Hearts);
21+
} catch (Error $e) {
22+
echo $e->getMessage(), "\n";
23+
}
24+
25+
try {
26+
var_dump(Suit::from(42));
27+
} catch (Error $e) {
28+
echo $e->getMessage(), "\n";
29+
}
30+
31+
try {
32+
var_dump(Suit::tryFrom('bar'));
33+
} catch (Error $e) {
34+
echo $e->getMessage(), "\n";
35+
}
36+
1337
?>
14-
--EXPECTF--
15-
Fatal error: Duplicate value in enum Suit for cases Hearts and Spades in %s on line %s
38+
--EXPECT--
39+
Duplicate value in enum Suit for cases Hearts and Spades
40+
Duplicate value in enum Suit for cases Hearts and Spades
41+
Duplicate value in enum Suit for cases Hearts and Spades
42+
Duplicate value in enum Suit for cases Hearts and Spades

Zend/tests/enum/backed-int-const-invalid-expr.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,4 @@ var_dump(Foo::Bar->value);
1111

1212
?>
1313
--EXPECTF--
14-
Fatal error: Enum case value must be compile-time evaluatable in %s on line %d
14+
Fatal error: Constant expression contains invalid operations in %s on line %d

Zend/tests/enum/backed-mismatch.phpt

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,33 @@ enum Foo: int {
77
case Bar = 'bar';
88
}
99

10+
try {
11+
var_dump(Foo::Bar);
12+
} catch (Error $e) {
13+
echo get_class($e), ': ', $e->getMessage(), "\n";
14+
}
15+
16+
try {
17+
var_dump(Foo::Bar);
18+
} catch (Error $e) {
19+
echo get_class($e), ': ', $e->getMessage(), "\n";
20+
}
21+
22+
try {
23+
var_dump(Foo::from(42));
24+
} catch (Error $e) {
25+
echo get_class($e), ': ', $e->getMessage(), "\n";
26+
}
27+
28+
try {
29+
var_dump(Foo::from('bar'));
30+
} catch (Error $e) {
31+
echo get_class($e), ': ', $e->getMessage(), "\n";
32+
}
33+
1034
?>
11-
--EXPECTF--
12-
Fatal error: Enum case type string does not match enum backing type int in %s on line %d
35+
--EXPECT--
36+
TypeError: Enum case type string does not match enum backing type int
37+
TypeError: Enum case type string does not match enum backing type int
38+
TypeError: Enum case type string does not match enum backing type int
39+
TypeError: Foo::from(): Argument #1 ($value) must be of type int, string given

Zend/tests/enum/gh7821.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-7821: Can't use arbitrary constant expressions in enum cases
3+
--FILE--
4+
<?php
5+
6+
interface I {
7+
public const A = 'A';
8+
public const B = 'B';
9+
}
10+
11+
enum B: string implements I {
12+
case C = I::A;
13+
case D = self::B;
14+
}
15+
16+
var_dump(B::A);
17+
var_dump(B::B);
18+
var_dump(B::C->value);
19+
var_dump(B::D->value);
20+
21+
?>
22+
--EXPECT--
23+
string(1) "A"
24+
string(1) "B"
25+
string(1) "A"
26+
string(1) "B"

Zend/tests/enum/gh8418.phpt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
GH-8418: Enum constant expression evaluation doesn't work with warmed cache
3+
--FILE--
4+
<?php
5+
6+
class ExampleClass
7+
{
8+
public const EXAMPLE_CONST = 42;
9+
}
10+
11+
enum ExampleEnum: int
12+
{
13+
case ENUM_CASE = ExampleClass::EXAMPLE_CONST;
14+
}
15+
16+
var_dump(ExampleEnum::ENUM_CASE->value);
17+
18+
?>
19+
--EXPECT--
20+
int(42)

Zend/tests/enum/non-backed-enum-with-expr-value.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ enum Foo {
99

1010
?>
1111
--EXPECTF--
12-
Fatal error: Case Bar of non-backed enum Foo must not have a value, try adding ": int" to the enum declaration in %s on line %d
12+
Fatal error: Case Bar of non-backed enum Foo must not have a value in %s on line %d

Zend/tests/enum/non-backed-enum-with-int-value.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ enum Foo {
99

1010
?>
1111
--EXPECTF--
12-
Fatal error: Case Bar of non-backed enum Foo must not have a value, try adding ": int" to the enum declaration in %s on line %d
12+
Fatal error: Case Bar of non-backed enum Foo must not have a value in %s on line %d

Zend/tests/enum/non-backed-enum-with-string-value.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ enum Foo {
99

1010
?>
1111
--EXPECTF--
12-
Fatal error: Case Bar of non-backed enum Foo must not have a value, try adding ": string" to the enum declaration in %s on line %d
12+
Fatal error: Case Bar of non-backed enum Foo must not have a value in %s on line %d
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
Test failure of updating class constants
3+
--FILE--
4+
<?php
5+
6+
enum Foo: string {
7+
const Bar = NONEXISTENT;
8+
}
9+
10+
var_dump(Foo::Bar);
11+
12+
?>
13+
--EXPECTF--
14+
Fatal error: Uncaught Error: Undefined constant "NONEXISTENT" in %s:%d
15+
Stack trace:
16+
#0 {main}
17+
thrown in %s on line %d

Zend/zend_API.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "zend_closures.h"
3131
#include "zend_inheritance.h"
3232
#include "zend_ini.h"
33+
#include "zend_enum.h"
3334

3435
#include <stdarg.h>
3536

@@ -1493,6 +1494,12 @@ ZEND_API zend_result zend_update_class_constants(zend_class_entry *class_type) /
14931494
}
14941495
}
14951496

1497+
if (class_type->type == ZEND_USER_CLASS && class_type->ce_flags & ZEND_ACC_ENUM && class_type->enum_backing_type != IS_UNDEF) {
1498+
if (zend_enum_build_backed_enum_table(class_type) == FAILURE) {
1499+
return FAILURE;
1500+
}
1501+
}
1502+
14961503
ce_flags |= ZEND_ACC_CONSTANTS_UPDATED;
14971504
ce_flags &= ~ZEND_ACC_HAS_AST_CONSTANTS;
14981505
ce_flags &= ~ZEND_ACC_HAS_AST_PROPERTIES;

Zend/zend_ast.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -776,12 +776,18 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast
776776
zend_string *case_name = zend_ast_get_str(case_name_ast);
777777

778778
zend_ast *case_value_ast = ast->child[2];
779-
zval *case_value_zv = case_value_ast != NULL
780-
? zend_ast_get_zval(case_value_ast)
781-
: NULL;
779+
780+
zval case_value_zv;
781+
ZVAL_UNDEF(&case_value_zv);
782+
if (case_value_ast != NULL) {
783+
if (UNEXPECTED(zend_ast_evaluate(&case_value_zv, case_value_ast, scope) != SUCCESS)) {
784+
return FAILURE;
785+
}
786+
}
782787

783788
zend_class_entry *ce = zend_lookup_class(class_name);
784-
zend_enum_new(result, ce, case_name, case_value_zv);
789+
zend_enum_new(result, ce, case_name, case_value_ast != NULL ? &case_value_zv : NULL);
790+
zval_ptr_dtor_nogc(&case_value_zv);
785791
break;
786792
}
787793
case ZEND_AST_CLASS_CONST:

Zend/zend_compile.c

Lines changed: 7 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -7578,9 +7578,6 @@ static void zend_compile_enum_backing_type(zend_class_entry *ce, zend_ast *enum_
75787578
ce->enum_backing_type = IS_STRING;
75797579
}
75807580
zend_type_release(type, 0);
7581-
7582-
ce->backed_enum_table = emalloc(sizeof(HashTable));
7583-
zend_hash_init(ce->backed_enum_table, 0, NULL, ZVAL_PTR_DTOR, 0);
75847581
}
75857582

75867583
static void zend_compile_class_decl(znode *result, zend_ast *ast, bool toplevel) /* {{{ */
@@ -7792,69 +7789,20 @@ static void zend_compile_enum_case(zend_ast *ast)
77927789
ZVAL_STR_COPY(&case_name_zval, enum_case_name);
77937790
zend_ast *case_name_ast = zend_ast_create_zval(&case_name_zval);
77947791

7795-
zend_ast *case_value_zval_ast = NULL;
77967792
zend_ast *case_value_ast = ast->child[1];
7793+
// Remove case_value_ast from the original AST to avoid freeing it, as it will be freed by zend_const_expr_to_zval
7794+
ast->child[1] = NULL;
77977795
if (enum_class->enum_backing_type != IS_UNDEF && case_value_ast == NULL) {
77987796
zend_error_noreturn(E_COMPILE_ERROR, "Case %s of backed enum %s must have a value",
77997797
ZSTR_VAL(enum_case_name),
78007798
ZSTR_VAL(enum_class_name));
7801-
}
7802-
if (case_value_ast != NULL) {
7803-
zend_eval_const_expr(&ast->child[1]);
7804-
case_value_ast = ast->child[1];
7805-
if (case_value_ast->kind != ZEND_AST_ZVAL) {
7806-
zend_error_noreturn(
7807-
E_COMPILE_ERROR, "Enum case value must be compile-time evaluatable");
7808-
}
7809-
7810-
zval case_value_zv;
7811-
ZVAL_COPY(&case_value_zv, zend_ast_get_zval(case_value_ast));
7812-
if (enum_class->enum_backing_type == IS_UNDEF) {
7813-
if (Z_TYPE(case_value_zv) == IS_LONG || Z_TYPE(case_value_zv) == IS_STRING) {
7814-
zend_error_noreturn(E_COMPILE_ERROR, "Case %s of non-backed enum %s must not have a value, try adding \": %s\" to the enum declaration",
7815-
ZSTR_VAL(enum_case_name),
7816-
ZSTR_VAL(enum_class_name),
7817-
zend_zval_type_name(&case_value_zv));
7818-
} else {
7819-
zend_error_noreturn(E_COMPILE_ERROR, "Case %s of non-backed enum %s must not have a value",
7820-
ZSTR_VAL(enum_case_name),
7821-
ZSTR_VAL(enum_class_name));
7822-
}
7823-
}
7824-
7825-
if (enum_class->enum_backing_type != Z_TYPE(case_value_zv)) {
7826-
zend_error_noreturn(E_COMPILE_ERROR, "Enum case type %s does not match enum backing type %s",
7827-
zend_get_type_by_const(Z_TYPE(case_value_zv)),
7828-
zend_get_type_by_const(enum_class->enum_backing_type));
7829-
}
7830-
7831-
case_value_zval_ast = zend_ast_create_zval(&case_value_zv);
7832-
Z_TRY_ADDREF(case_name_zval);
7833-
if (enum_class->enum_backing_type == IS_LONG) {
7834-
zend_long long_key = Z_LVAL(case_value_zv);
7835-
zval *existing_case_name = zend_hash_index_find(enum_class->backed_enum_table, long_key);
7836-
if (existing_case_name) {
7837-
zend_error_noreturn(E_COMPILE_ERROR, "Duplicate value in enum %s for cases %s and %s",
7838-
ZSTR_VAL(enum_class_name),
7839-
Z_STRVAL_P(existing_case_name),
7840-
ZSTR_VAL(enum_case_name));
7841-
}
7842-
zend_hash_index_add_new(enum_class->backed_enum_table, long_key, &case_name_zval);
7843-
} else {
7844-
ZEND_ASSERT(enum_class->enum_backing_type == IS_STRING);
7845-
zend_string *string_key = Z_STR(case_value_zv);
7846-
zval *existing_case_name = zend_hash_find(enum_class->backed_enum_table, string_key);
7847-
if (existing_case_name != NULL) {
7848-
zend_error_noreturn(E_COMPILE_ERROR, "Duplicate value in enum %s for cases %s and %s",
7849-
ZSTR_VAL(enum_class_name),
7850-
Z_STRVAL_P(existing_case_name),
7851-
ZSTR_VAL(enum_case_name));
7852-
}
7853-
zend_hash_add_new(enum_class->backed_enum_table, string_key, &case_name_zval);
7854-
}
7799+
} else if (enum_class->enum_backing_type == IS_UNDEF && case_value_ast != NULL) {
7800+
zend_error_noreturn(E_COMPILE_ERROR, "Case %s of non-backed enum %s must not have a value",
7801+
ZSTR_VAL(enum_case_name),
7802+
ZSTR_VAL(enum_class_name));
78557803
}
78567804

7857-
zend_ast *const_enum_init_ast = zend_ast_create(ZEND_AST_CONST_ENUM_INIT, class_name_ast, case_name_ast, case_value_zval_ast);
7805+
zend_ast *const_enum_init_ast = zend_ast_create(ZEND_AST_CONST_ENUM_INIT, class_name_ast, case_name_ast, case_value_ast);
78587806

78597807
zval value_zv;
78607808
zend_const_expr_to_zval(&value_zv, &const_enum_init_ast, /* allow_dynamic */ false);

0 commit comments

Comments
 (0)