Skip to content

Commit 186e573

Browse files
iluuu1994iluuu1994
committed
Allow arbitrary const expressions in backed enums
Closes GH-7821
1 parent ddb04d4 commit 186e573

15 files changed

+146
-96
lines changed

‎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/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/name-property.phpt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ var_dump(IntFoo::Bar->name);
2323
--EXPECT--
2424
array(1) {
2525
[0]=>
26-
object(ReflectionProperty)#2 (2) {
26+
object(ReflectionProperty)#4 (2) {
2727
["name"]=>
2828
string(4) "name"
2929
["class"]=>
@@ -33,14 +33,14 @@ array(1) {
3333
string(3) "Bar"
3434
array(2) {
3535
[0]=>
36-
object(ReflectionProperty)#3 (2) {
36+
object(ReflectionProperty)#5 (2) {
3737
["name"]=>
3838
string(4) "name"
3939
["class"]=>
4040
string(6) "IntFoo"
4141
}
4242
[1]=>
43-
object(ReflectionProperty)#4 (2) {
43+
object(ReflectionProperty)#6 (2) {
4444
["name"]=>
4545
string(5) "value"
4646
["class"]=>

‎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

‎Zend/tests/enum/test.phpt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
GH-7821: Can't use arbitrary constant expressiosn in enum cases
3+
--FILE--
4+
<?php
5+
6+
interface I {
7+
public const A = 'A';
8+
}
9+
10+
class B implements I {
11+
const B = self::A;
12+
}
13+
14+
new B();
15+
16+
?>
17+
--EXPECT--

‎Zend/zend_ast.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -770,9 +770,16 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast
770770
zend_string *case_name = zend_ast_get_str(case_name_ast);
771771

772772
zend_ast *case_value_ast = ast->child[2];
773-
zval *case_value_zv = case_value_ast != NULL
774-
? zend_ast_get_zval(case_value_ast)
775-
: NULL;
773+
774+
zval case_value_zv;
775+
zval *case_value_zv_ptr = NULL;
776+
if (case_value_ast != NULL) {
777+
if (UNEXPECTED(zend_ast_evaluate(&case_value_zv, case_value_ast, scope) != SUCCESS)) {
778+
ret = FAILURE;
779+
break;
780+
}
781+
case_value_zv_ptr = &case_value_zv;
782+
}
776783

777784
zend_class_entry *ce = zend_lookup_class(class_name);
778785
if (!ce) {
@@ -782,7 +789,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast
782789
return FAILURE;
783790
}
784791

785-
zend_enum_new(result, ce, case_name, case_value_zv);
792+
zend_enum_new(result, ce, case_name, case_value_zv_ptr);
786793
break;
787794
}
788795
case ZEND_AST_CLASS_CONST:

‎Zend/zend_compile.c

Lines changed: 5 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -7571,9 +7571,6 @@ static void zend_compile_enum_backing_type(zend_class_entry *ce, zend_ast *enum_
75717571
ce->enum_backing_type = IS_STRING;
75727572
}
75737573
zend_type_release(type, 0);
7574-
7575-
ce->backed_enum_table = emalloc(sizeof(HashTable));
7576-
zend_hash_init(ce->backed_enum_table, 0, NULL, ZVAL_PTR_DTOR, 0);
75777574
}
75787575

75797576
static void zend_compile_class_decl(znode *result, zend_ast *ast, bool toplevel) /* {{{ */
@@ -7785,69 +7782,18 @@ static void zend_compile_enum_case(zend_ast *ast)
77857782
ZVAL_STR_COPY(&case_name_zval, enum_case_name);
77867783
zend_ast *case_name_ast = zend_ast_create_zval(&case_name_zval);
77877784

7788-
zend_ast *case_value_zval_ast = NULL;
77897785
zend_ast *case_value_ast = ast->child[1];
77907786
if (enum_class->enum_backing_type != IS_UNDEF && case_value_ast == NULL) {
77917787
zend_error_noreturn(E_COMPILE_ERROR, "Case %s of backed enum %s must have a value",
77927788
ZSTR_VAL(enum_case_name),
77937789
ZSTR_VAL(enum_class_name));
7794-
}
7795-
if (case_value_ast != NULL) {
7796-
zend_eval_const_expr(&ast->child[1]);
7797-
case_value_ast = ast->child[1];
7798-
if (case_value_ast->kind != ZEND_AST_ZVAL) {
7799-
zend_error_noreturn(
7800-
E_COMPILE_ERROR, "Enum case value must be compile-time evaluatable");
7801-
}
7802-
7803-
zval case_value_zv;
7804-
ZVAL_COPY(&case_value_zv, zend_ast_get_zval(case_value_ast));
7805-
if (enum_class->enum_backing_type == IS_UNDEF) {
7806-
if (Z_TYPE(case_value_zv) == IS_LONG || Z_TYPE(case_value_zv) == IS_STRING) {
7807-
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",
7808-
ZSTR_VAL(enum_case_name),
7809-
ZSTR_VAL(enum_class_name),
7810-
zend_zval_type_name(&case_value_zv));
7811-
} else {
7812-
zend_error_noreturn(E_COMPILE_ERROR, "Case %s of non-backed enum %s must not have a value",
7813-
ZSTR_VAL(enum_case_name),
7814-
ZSTR_VAL(enum_class_name));
7815-
}
7816-
}
7817-
7818-
if (enum_class->enum_backing_type != Z_TYPE(case_value_zv)) {
7819-
zend_error_noreturn(E_COMPILE_ERROR, "Enum case type %s does not match enum backing type %s",
7820-
zend_get_type_by_const(Z_TYPE(case_value_zv)),
7821-
zend_get_type_by_const(enum_class->enum_backing_type));
7822-
}
7823-
7824-
case_value_zval_ast = zend_ast_create_zval(&case_value_zv);
7825-
Z_TRY_ADDREF(case_name_zval);
7826-
if (enum_class->enum_backing_type == IS_LONG) {
7827-
zend_long long_key = Z_LVAL(case_value_zv);
7828-
zval *existing_case_name = zend_hash_index_find(enum_class->backed_enum_table, long_key);
7829-
if (existing_case_name) {
7830-
zend_error_noreturn(E_COMPILE_ERROR, "Duplicate value in enum %s for cases %s and %s",
7831-
ZSTR_VAL(enum_class_name),
7832-
Z_STRVAL_P(existing_case_name),
7833-
ZSTR_VAL(enum_case_name));
7834-
}
7835-
zend_hash_index_add_new(enum_class->backed_enum_table, long_key, &case_name_zval);
7836-
} else {
7837-
ZEND_ASSERT(enum_class->enum_backing_type == IS_STRING);
7838-
zend_string *string_key = Z_STR(case_value_zv);
7839-
zval *existing_case_name = zend_hash_find(enum_class->backed_enum_table, string_key);
7840-
if (existing_case_name != NULL) {
7841-
zend_error_noreturn(E_COMPILE_ERROR, "Duplicate value in enum %s for cases %s and %s",
7842-
ZSTR_VAL(enum_class_name),
7843-
Z_STRVAL_P(existing_case_name),
7844-
ZSTR_VAL(enum_case_name));
7845-
}
7846-
zend_hash_add_new(enum_class->backed_enum_table, string_key, &case_name_zval);
7847-
}
7790+
} else if (enum_class->enum_backing_type == IS_UNDEF && case_value_ast != NULL) {
7791+
zend_error_noreturn(E_COMPILE_ERROR, "Case %s of non-backed enum %s must not have a value",
7792+
ZSTR_VAL(enum_case_name),
7793+
ZSTR_VAL(enum_class_name));
78487794
}
78497795

7850-
zend_ast *const_enum_init_ast = zend_ast_create(ZEND_AST_CONST_ENUM_INIT, class_name_ast, case_name_ast, case_value_zval_ast);
7796+
zend_ast *const_enum_init_ast = zend_ast_create(ZEND_AST_CONST_ENUM_INIT, class_name_ast, case_name_ast, case_value_ast);
78517797

78527798
zval value_zv;
78537799
zend_const_expr_to_zval(&value_zv, &const_enum_init_ast, /* allow_dynamic */ false);

‎Zend/zend_constants.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,8 @@ ZEND_API zval *zend_get_class_constant_ex(zend_string *class_name, zend_string *
330330
zend_class_constant *c = NULL;
331331
zval *ret_constant = NULL;
332332

333-
if (ZSTR_HAS_CE_CACHE(class_name)) {
333+
// FIXME: Why does "self" have the CE cache?
334+
if (false && ZSTR_HAS_CE_CACHE(class_name)) {
334335
ce = ZSTR_GET_CE_CACHE(class_name);
335336
if (!ce) {
336337
ce = zend_fetch_class(class_name, flags);

‎Zend/zend_enum.c

Lines changed: 67 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "zend_compile.h"
2222
#include "zend_enum_arginfo.h"
2323
#include "zend_interfaces.h"
24+
#include "zend_enum.h"
2425

2526
#define ZEND_ENUM_DISALLOW_MAGIC_METHOD(propertyName, methodName) \
2627
do { \
@@ -183,6 +184,65 @@ void zend_enum_add_interfaces(zend_class_entry *ce)
183184
}
184185
}
185186

187+
void zend_enum_build_backed_enum_table(zend_class_entry *ce)
188+
{
189+
uint32_t backing_type = ce->enum_backing_type;
190+
ZEND_ASSERT(backing_type != IS_UNDEF);
191+
192+
if (zend_update_class_constants(ce) == FAILURE) {
193+
// FIXME: What's the right way to error here?
194+
zend_error_noreturn(E_COMPILE_ERROR, "Could not update class constants");
195+
}
196+
197+
ce->backed_enum_table = emalloc(sizeof(HashTable));
198+
zend_hash_init(ce->backed_enum_table, 0, NULL, ZVAL_PTR_DTOR, 0);
199+
200+
zend_string *enum_class_name = ce->name;
201+
202+
zend_string *name;
203+
zval *val;
204+
ZEND_HASH_MAP_FOREACH_STR_KEY_VAL(&ce->constants_table, name, val) {
205+
zend_class_constant *c = Z_PTR_P(val);
206+
if ((ZEND_CLASS_CONST_FLAGS(c) & ZEND_CLASS_CONST_IS_CASE) == 0) {
207+
continue;
208+
}
209+
210+
zval *c_value = &c->value;
211+
zval *case_value = zend_enum_fetch_case_value(Z_OBJ_P(c_value));
212+
213+
if (ce->enum_backing_type != Z_TYPE_P(case_value)) {
214+
zend_error_noreturn(E_COMPILE_ERROR, "Enum case type %s does not match enum backing type %s",
215+
zend_get_type_by_const(Z_TYPE_P(case_value)),
216+
zend_get_type_by_const(ce->enum_backing_type));
217+
}
218+
219+
if (ce->enum_backing_type == IS_LONG) {
220+
zend_long long_key = Z_LVAL_P(case_value);
221+
zval *existing_case_object = zend_hash_index_find(ce->backed_enum_table, long_key);
222+
if (existing_case_object) {
223+
zval *existing_case_name = zend_enum_fetch_case_name(Z_OBJ_P(existing_case_object));
224+
zend_error_noreturn(E_COMPILE_ERROR, "Duplicate value in enum %s for cases %s and %s",
225+
ZSTR_VAL(enum_class_name),
226+
Z_STRVAL_P(existing_case_name),
227+
ZSTR_VAL(name));
228+
}
229+
zend_hash_index_add_new(ce->backed_enum_table, long_key, c_value);
230+
} else {
231+
ZEND_ASSERT(ce->enum_backing_type == IS_STRING);
232+
zend_string *string_key = Z_STR_P(case_value);
233+
zval *existing_case_object = zend_hash_find(ce->backed_enum_table, string_key);
234+
if (existing_case_object != NULL) {
235+
zval *existing_case_name = zend_enum_fetch_case_name(Z_OBJ_P(existing_case_object));
236+
zend_error_noreturn(E_COMPILE_ERROR, "Duplicate value in enum %s for cases %s and %s",
237+
ZSTR_VAL(enum_class_name),
238+
Z_STRVAL_P(existing_case_name),
239+
ZSTR_VAL(name));
240+
}
241+
zend_hash_add_new(ce->backed_enum_table, string_key, c_value);
242+
}
243+
} ZEND_HASH_FOREACH_END();
244+
}
245+
186246
static ZEND_NAMED_FUNCTION(zend_enum_cases_func)
187247
{
188248
zend_class_entry *ce = execute_data->func->common.scope;
@@ -213,23 +273,23 @@ static void zend_enum_from_base(INTERNAL_FUNCTION_PARAMETERS, bool try)
213273
zend_string *string_key;
214274
zend_long long_key;
215275

216-
zval *case_name_zv;
276+
zval *case_value;
217277
if (ce->enum_backing_type == IS_LONG) {
218278
ZEND_PARSE_PARAMETERS_START(1, 1)
219279
Z_PARAM_LONG(long_key)
220280
ZEND_PARSE_PARAMETERS_END();
221281

222-
case_name_zv = zend_hash_index_find(ce->backed_enum_table, long_key);
282+
case_value = zend_hash_index_find(ce->backed_enum_table, long_key);
223283
} else {
224284
ZEND_PARSE_PARAMETERS_START(1, 1)
225285
Z_PARAM_STR(string_key)
226286
ZEND_PARSE_PARAMETERS_END();
227287

228288
ZEND_ASSERT(ce->enum_backing_type == IS_STRING);
229-
case_name_zv = zend_hash_find(ce->backed_enum_table, string_key);
289+
case_value = zend_hash_find(ce->backed_enum_table, string_key);
230290
}
231291

232-
if (case_name_zv == NULL) {
292+
if (case_value == NULL) {
233293
if (try) {
234294
RETURN_NULL();
235295
}
@@ -243,19 +303,7 @@ static void zend_enum_from_base(INTERNAL_FUNCTION_PARAMETERS, bool try)
243303
RETURN_THROWS();
244304
}
245305

246-
// TODO: We might want to store pointers to constants in backed_enum_table instead of names,
247-
// to make this lookup more efficient.
248-
ZEND_ASSERT(Z_TYPE_P(case_name_zv) == IS_STRING);
249-
zend_class_constant *c = zend_hash_find_ptr(CE_CONSTANTS_TABLE(ce), Z_STR_P(case_name_zv));
250-
ZEND_ASSERT(c != NULL);
251-
zval *case_zv = &c->value;
252-
if (Z_TYPE_P(case_zv) == IS_CONSTANT_AST) {
253-
if (zval_update_constant_ex(case_zv, c->ce) == FAILURE) {
254-
RETURN_THROWS();
255-
}
256-
}
257-
258-
ZVAL_COPY(return_value, case_zv);
306+
ZVAL_COPY(return_value, case_value);
259307
}
260308

261309
static ZEND_NAMED_FUNCTION(zend_enum_from_func)
@@ -434,9 +482,9 @@ ZEND_API void zend_enum_add_case(zend_class_entry *ce, zend_string *case_name, z
434482
zval case_name_zv;
435483
ZVAL_STR(&case_name_zv, case_name);
436484
if (Z_TYPE_P(value) == IS_LONG) {
437-
zend_hash_index_add_new(ce->backed_enum_table, Z_LVAL_P(value), &case_name_zv);
485+
zend_hash_index_add_new(ce->backed_enum_table, Z_LVAL_P(value), value);
438486
} else {
439-
zend_hash_add_new(ce->backed_enum_table, Z_STR_P(value), &case_name_zv);
487+
zend_hash_add_new(ce->backed_enum_table, Z_STR_P(value), value);
440488
}
441489
} else {
442490
ZEND_ASSERT(ce->enum_backing_type == IS_UNDEF);

‎Zend/zend_enum.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ extern ZEND_API zend_class_entry *zend_ce_backed_enum;
2929

3030
void zend_register_enum_ce(void);
3131
void zend_enum_add_interfaces(zend_class_entry *ce);
32+
void zend_enum_build_backed_enum_table(zend_class_entry *ce);
3233
zend_object *zend_enum_new(zval *result, zend_class_entry *ce, zend_string *case_name, zval *backing_value_zv);
3334
void zend_verify_enum(zend_class_entry *ce);
3435
void zend_enum_register_funcs(zend_class_entry *ce);

‎Zend/zend_inheritance.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2906,6 +2906,10 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
29062906
ZSTR_SET_CE_CACHE(ce->name, ce);
29072907
}
29082908

2909+
if (ce->ce_flags & ZEND_ACC_ENUM && ce->enum_backing_type != IS_UNDEF) {
2910+
zend_enum_build_backed_enum_table(ce);
2911+
}
2912+
29092913
return ce;
29102914
}
29112915
/* }}} */

‎ext/reflection/tests/ReflectionEnum_getCase.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ test(IntEnum::class, 'Baz');
3232

3333
?>
3434
--EXPECT--
35-
object(ReflectionEnumUnitCase)#2 (2) {
35+
object(ReflectionEnumUnitCase)#3 (2) {
3636
["name"]=>
3737
string(3) "Foo"
3838
["class"]=>
3939
string(5) "Enum_"
4040
}
4141
ReflectionException: Enum_::Bar is not a case
4242
ReflectionException: Case Enum_::Baz does not exist
43-
object(ReflectionEnumBackedCase)#2 (2) {
43+
object(ReflectionEnumBackedCase)#3 (2) {
4444
["name"]=>
4545
string(3) "Foo"
4646
["class"]=>

0 commit comments

Comments
 (0)