Allow arbitrary const expressions in backed enums

iluuu1994 · iluuu1994 · commit 186e5730302a · 2022-03-11T12:10:34.000+01:00
Closes GH-7821
diff --git a/Zend/tests/enum/backed-int-const-invalid-expr.phpt b/Zend/tests/enum/backed-int-const-invalid-expr.phpt
@@ -11,4 +11,4 @@ var_dump(Foo::Bar->value);
 
 ?>
 --EXPECTF--
-Fatal error: Enum case value must be compile-time evaluatable in %s on line %d
+Fatal error: Constant expression contains invalid operations in %s on line %d
diff --git a/Zend/tests/enum/gh7821.phpt b/Zend/tests/enum/gh7821.phpt
@@ -0,0 +1,26 @@
+--TEST--
+GH-7821: Can't use arbitrary constant expressions in enum cases
+--FILE--
+<?php
+
+interface I {
+    public const A = 'A';
+    public const B = 'B';
+}
+
+enum B: string implements I {
+    case C = I::A;
+    case D = self::B;
+}
+
+var_dump(B::A);
+var_dump(B::B);
+var_dump(B::C->value);
+var_dump(B::D->value);
+
+?>
+--EXPECT--
+string(1) "A"
+string(1) "B"
+string(1) "A"
+string(1) "B"
diff --git a/Zend/tests/enum/name-property.phpt b/Zend/tests/enum/name-property.phpt
@@ -23,7 +23,7 @@ var_dump(IntFoo::Bar->name);
 --EXPECT--
 array(1) {
   [0]=>
-  object(ReflectionProperty)#2 (2) {
+  object(ReflectionProperty)#4 (2) {
     ["name"]=>
     string(4) "name"
     ["class"]=>
@@ -33,14 +33,14 @@ array(1) {
 string(3) "Bar"
 array(2) {
   [0]=>
-  object(ReflectionProperty)#3 (2) {
+  object(ReflectionProperty)#5 (2) {
     ["name"]=>
     string(4) "name"
     ["class"]=>
     string(6) "IntFoo"
   }
   [1]=>
-  object(ReflectionProperty)#4 (2) {
+  object(ReflectionProperty)#6 (2) {
     ["name"]=>
     string(5) "value"
     ["class"]=>
diff --git a/Zend/tests/enum/non-backed-enum-with-expr-value.phpt b/Zend/tests/enum/non-backed-enum-with-expr-value.phpt
@@ -9,4 +9,4 @@ enum Foo {
 
 ?>
 --EXPECTF--
-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
+Fatal error: Case Bar of non-backed enum Foo must not have a value in %s on line %d
diff --git a/Zend/tests/enum/non-backed-enum-with-int-value.phpt b/Zend/tests/enum/non-backed-enum-with-int-value.phpt
@@ -9,4 +9,4 @@ enum Foo {
 
 ?>
 --EXPECTF--
-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
+Fatal error: Case Bar of non-backed enum Foo must not have a value in %s on line %d
diff --git a/Zend/tests/enum/non-backed-enum-with-string-value.phpt b/Zend/tests/enum/non-backed-enum-with-string-value.phpt
@@ -9,4 +9,4 @@ enum Foo {
 
 ?>
 --EXPECTF--
-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
+Fatal error: Case Bar of non-backed enum Foo must not have a value in %s on line %d
diff --git a/Zend/tests/enum/test.phpt b/Zend/tests/enum/test.phpt
@@ -0,0 +1,17 @@
+--TEST--
+GH-7821: Can't use arbitrary constant expressiosn in enum cases
+--FILE--
+<?php
+
+interface I {
+    public const A = 'A';
+}
+
+class B implements I {
+    const B = self::A;
+}
+
+new B();
+
+?>
+--EXPECT--
diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c
@@ -770,9 +770,16 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast
 			zend_string *case_name = zend_ast_get_str(case_name_ast);
 
 			zend_ast *case_value_ast = ast->child[2];
-			zval *case_value_zv = case_value_ast != NULL
-				? zend_ast_get_zval(case_value_ast)
-				: NULL;
+
+			zval case_value_zv;
+			zval *case_value_zv_ptr = NULL;
+			if (case_value_ast != NULL) {
+				if (UNEXPECTED(zend_ast_evaluate(&case_value_zv, case_value_ast, scope) != SUCCESS)) {
+					ret = FAILURE;
+					break;
+				}
+				case_value_zv_ptr = &case_value_zv;
+			}
 
 			zend_class_entry *ce = zend_lookup_class(class_name);
 			if (!ce) {
@@ -782,7 +789,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_ast_evaluate(zval *result, zend_ast *ast
 				return FAILURE;
 			}
 
-			zend_enum_new(result, ce, case_name, case_value_zv);
+			zend_enum_new(result, ce, case_name, case_value_zv_ptr);
 			break;
 		}
 		case ZEND_AST_CLASS_CONST:
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -7571,9 +7571,6 @@ static void zend_compile_enum_backing_type(zend_class_entry *ce, zend_ast *enum_
 		ce->enum_backing_type = IS_STRING;
 	}
 	zend_type_release(type, 0);
-
-	ce->backed_enum_table = emalloc(sizeof(HashTable));
-	zend_hash_init(ce->backed_enum_table, 0, NULL, ZVAL_PTR_DTOR, 0);
 }
 
 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)
 	ZVAL_STR_COPY(&case_name_zval, enum_case_name);
 	zend_ast *case_name_ast = zend_ast_create_zval(&case_name_zval);
 
-	zend_ast *case_value_zval_ast = NULL;
 	zend_ast *case_value_ast = ast->child[1];
 	if (enum_class->enum_backing_type != IS_UNDEF && case_value_ast == NULL) {
 		zend_error_noreturn(E_COMPILE_ERROR, "Case %s of backed enum %s must have a value",
 			ZSTR_VAL(enum_case_name),
 			ZSTR_VAL(enum_class_name));
-	}
-	if (case_value_ast != NULL) {
-		zend_eval_const_expr(&ast->child[1]);
-		case_value_ast = ast->child[1];
-		if (case_value_ast->kind != ZEND_AST_ZVAL) {
-			zend_error_noreturn(
-				E_COMPILE_ERROR, "Enum case value must be compile-time evaluatable");
-		}
-
-		zval case_value_zv;
-		ZVAL_COPY(&case_value_zv, zend_ast_get_zval(case_value_ast));
-		if (enum_class->enum_backing_type == IS_UNDEF) {
-			if (Z_TYPE(case_value_zv) == IS_LONG || Z_TYPE(case_value_zv) == IS_STRING) {
-				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",
-					ZSTR_VAL(enum_case_name),
-					ZSTR_VAL(enum_class_name),
-					zend_zval_type_name(&case_value_zv));
-			} else {
-				zend_error_noreturn(E_COMPILE_ERROR, "Case %s of non-backed enum %s must not have a value",
-					ZSTR_VAL(enum_case_name),
-					ZSTR_VAL(enum_class_name));
-			}
-		}
-
-		if (enum_class->enum_backing_type != Z_TYPE(case_value_zv)) {
-			zend_error_noreturn(E_COMPILE_ERROR, "Enum case type %s does not match enum backing type %s",
-				zend_get_type_by_const(Z_TYPE(case_value_zv)),
-				zend_get_type_by_const(enum_class->enum_backing_type));
-		}
-
-		case_value_zval_ast = zend_ast_create_zval(&case_value_zv);
-		Z_TRY_ADDREF(case_name_zval);
-		if (enum_class->enum_backing_type == IS_LONG) {
-			zend_long long_key = Z_LVAL(case_value_zv);
-			zval *existing_case_name = zend_hash_index_find(enum_class->backed_enum_table, long_key);
-			if (existing_case_name) {
-				zend_error_noreturn(E_COMPILE_ERROR, "Duplicate value in enum %s for cases %s and %s",
-					ZSTR_VAL(enum_class_name),
-					Z_STRVAL_P(existing_case_name),
-					ZSTR_VAL(enum_case_name));
-			}
-			zend_hash_index_add_new(enum_class->backed_enum_table, long_key, &case_name_zval);
-		} else {
-			ZEND_ASSERT(enum_class->enum_backing_type == IS_STRING);
-			zend_string *string_key = Z_STR(case_value_zv);
-			zval *existing_case_name = zend_hash_find(enum_class->backed_enum_table, string_key);
-			if (existing_case_name != NULL) {
-				zend_error_noreturn(E_COMPILE_ERROR, "Duplicate value in enum %s for cases %s and %s",
-					ZSTR_VAL(enum_class_name),
-					Z_STRVAL_P(existing_case_name),
-					ZSTR_VAL(enum_case_name));
-			}
-			zend_hash_add_new(enum_class->backed_enum_table, string_key, &case_name_zval);
-		}
+	} else if (enum_class->enum_backing_type == IS_UNDEF && case_value_ast != NULL) {
+		zend_error_noreturn(E_COMPILE_ERROR, "Case %s of non-backed enum %s must not have a value",
+			ZSTR_VAL(enum_case_name),
+			ZSTR_VAL(enum_class_name));
 	}
 
-	zend_ast *const_enum_init_ast = zend_ast_create(ZEND_AST_CONST_ENUM_INIT, class_name_ast, case_name_ast, case_value_zval_ast);
+	zend_ast *const_enum_init_ast = zend_ast_create(ZEND_AST_CONST_ENUM_INIT, class_name_ast, case_name_ast, case_value_ast);
 
 	zval value_zv;
 	zend_const_expr_to_zval(&value_zv, &const_enum_init_ast, /* allow_dynamic */ false);
diff --git a/Zend/zend_constants.c b/Zend/zend_constants.c
@@ -330,7 +330,8 @@ ZEND_API zval *zend_get_class_constant_ex(zend_string *class_name, zend_string *
 	zend_class_constant *c = NULL;
 	zval *ret_constant = NULL;
 
-	if (ZSTR_HAS_CE_CACHE(class_name)) {
+	// FIXME: Why does "self" have the CE cache?
+	if (false && ZSTR_HAS_CE_CACHE(class_name)) {
 		ce = ZSTR_GET_CE_CACHE(class_name);
 		if (!ce) {
 			ce = zend_fetch_class(class_name, flags);
diff --git a/Zend/zend_enum.c b/Zend/zend_enum.c
@@ -21,6 +21,7 @@
 #include "zend_compile.h"
 #include "zend_enum_arginfo.h"
 #include "zend_interfaces.h"
+#include "zend_enum.h"
 
 #define ZEND_ENUM_DISALLOW_MAGIC_METHOD(propertyName, methodName) \
 	do { \
@@ -183,6 +184,65 @@ void zend_enum_add_interfaces(zend_class_entry *ce)
 	}
 }
 
+void zend_enum_build_backed_enum_table(zend_class_entry *ce)
+{
+	uint32_t backing_type = ce->enum_backing_type;
+	ZEND_ASSERT(backing_type != IS_UNDEF);
+
+	if (zend_update_class_constants(ce) == FAILURE) {
+		// FIXME: What's the right way to error here?
+		zend_error_noreturn(E_COMPILE_ERROR, "Could not update class constants");
+	}
+
+	ce->backed_enum_table = emalloc(sizeof(HashTable));
+	zend_hash_init(ce->backed_enum_table, 0, NULL, ZVAL_PTR_DTOR, 0);
+
+	zend_string *enum_class_name = ce->name;
+
+	zend_string *name;
+	zval *val;
+	ZEND_HASH_MAP_FOREACH_STR_KEY_VAL(&ce->constants_table, name, val) {
+		zend_class_constant *c = Z_PTR_P(val);
+		if ((ZEND_CLASS_CONST_FLAGS(c) & ZEND_CLASS_CONST_IS_CASE) == 0) {
+			continue;
+		}
+
+		zval *c_value = &c->value;
+		zval *case_value = zend_enum_fetch_case_value(Z_OBJ_P(c_value));
+
+		if (ce->enum_backing_type != Z_TYPE_P(case_value)) {
+			zend_error_noreturn(E_COMPILE_ERROR, "Enum case type %s does not match enum backing type %s",
+				zend_get_type_by_const(Z_TYPE_P(case_value)),
+				zend_get_type_by_const(ce->enum_backing_type));
+		}
+
+		if (ce->enum_backing_type == IS_LONG) {
+			zend_long long_key = Z_LVAL_P(case_value);
+			zval *existing_case_object = zend_hash_index_find(ce->backed_enum_table, long_key);
+			if (existing_case_object) {
+				zval *existing_case_name = zend_enum_fetch_case_name(Z_OBJ_P(existing_case_object));
+				zend_error_noreturn(E_COMPILE_ERROR, "Duplicate value in enum %s for cases %s and %s",
+					ZSTR_VAL(enum_class_name),
+					Z_STRVAL_P(existing_case_name),
+					ZSTR_VAL(name));
+			}
+			zend_hash_index_add_new(ce->backed_enum_table, long_key, c_value);
+		} else {
+			ZEND_ASSERT(ce->enum_backing_type == IS_STRING);
+			zend_string *string_key = Z_STR_P(case_value);
+			zval *existing_case_object = zend_hash_find(ce->backed_enum_table, string_key);
+			if (existing_case_object != NULL) {
+				zval *existing_case_name = zend_enum_fetch_case_name(Z_OBJ_P(existing_case_object));
+				zend_error_noreturn(E_COMPILE_ERROR, "Duplicate value in enum %s for cases %s and %s",
+					ZSTR_VAL(enum_class_name),
+					Z_STRVAL_P(existing_case_name),
+					ZSTR_VAL(name));
+			}
+			zend_hash_add_new(ce->backed_enum_table, string_key, c_value);
+		}
+	} ZEND_HASH_FOREACH_END();
+}
+
 static ZEND_NAMED_FUNCTION(zend_enum_cases_func)
 {
 	zend_class_entry *ce = execute_data->func->common.scope;
@@ -213,23 +273,23 @@ static void zend_enum_from_base(INTERNAL_FUNCTION_PARAMETERS, bool try)
 	zend_string *string_key;
 	zend_long long_key;
 
-	zval *case_name_zv;
+	zval *case_value;
 	if (ce->enum_backing_type == IS_LONG) {
 		ZEND_PARSE_PARAMETERS_START(1, 1)
 			Z_PARAM_LONG(long_key)
 		ZEND_PARSE_PARAMETERS_END();
 
-		case_name_zv = zend_hash_index_find(ce->backed_enum_table, long_key);
+		case_value = zend_hash_index_find(ce->backed_enum_table, long_key);
 	} else {
 		ZEND_PARSE_PARAMETERS_START(1, 1)
 			Z_PARAM_STR(string_key)
 		ZEND_PARSE_PARAMETERS_END();
 
 		ZEND_ASSERT(ce->enum_backing_type == IS_STRING);
-		case_name_zv = zend_hash_find(ce->backed_enum_table, string_key);
+		case_value = zend_hash_find(ce->backed_enum_table, string_key);
 	}
 
-	if (case_name_zv == NULL) {
+	if (case_value == NULL) {
 		if (try) {
 			RETURN_NULL();
 		}
@@ -243,19 +303,7 @@ static void zend_enum_from_base(INTERNAL_FUNCTION_PARAMETERS, bool try)
 		RETURN_THROWS();
 	}
 
-	// TODO: We might want to store pointers to constants in backed_enum_table instead of names,
-	// to make this lookup more efficient.
-	ZEND_ASSERT(Z_TYPE_P(case_name_zv) == IS_STRING);
-	zend_class_constant *c = zend_hash_find_ptr(CE_CONSTANTS_TABLE(ce), Z_STR_P(case_name_zv));
-	ZEND_ASSERT(c != NULL);
-	zval *case_zv = &c->value;
-	if (Z_TYPE_P(case_zv) == IS_CONSTANT_AST) {
-		if (zval_update_constant_ex(case_zv, c->ce) == FAILURE) {
-			RETURN_THROWS();
-		}
-	}
-
-	ZVAL_COPY(return_value, case_zv);
+	ZVAL_COPY(return_value, case_value);
 }
 
 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
 		zval case_name_zv;
 		ZVAL_STR(&case_name_zv, case_name);
 		if (Z_TYPE_P(value) == IS_LONG) {
-			zend_hash_index_add_new(ce->backed_enum_table, Z_LVAL_P(value), &case_name_zv);
+			zend_hash_index_add_new(ce->backed_enum_table, Z_LVAL_P(value), value);
 		} else {
-			zend_hash_add_new(ce->backed_enum_table, Z_STR_P(value), &case_name_zv);
+			zend_hash_add_new(ce->backed_enum_table, Z_STR_P(value), value);
 		}
 	} else {
 		ZEND_ASSERT(ce->enum_backing_type == IS_UNDEF);
diff --git a/Zend/zend_enum.h b/Zend/zend_enum.h
@@ -29,6 +29,7 @@ extern ZEND_API zend_class_entry *zend_ce_backed_enum;
 
 void zend_register_enum_ce(void);
 void zend_enum_add_interfaces(zend_class_entry *ce);
+void zend_enum_build_backed_enum_table(zend_class_entry *ce);
 zend_object *zend_enum_new(zval *result, zend_class_entry *ce, zend_string *case_name, zval *backing_value_zv);
 void zend_verify_enum(zend_class_entry *ce);
 void zend_enum_register_funcs(zend_class_entry *ce);
diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c
@@ -2906,6 +2906,10 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
 		ZSTR_SET_CE_CACHE(ce->name, ce);
 	}
 
+	if (ce->ce_flags & ZEND_ACC_ENUM && ce->enum_backing_type != IS_UNDEF) {
+		zend_enum_build_backed_enum_table(ce);
+	}
+
 	return ce;
 }
 /* }}} */
diff --git a/ext/reflection/tests/ReflectionEnum_getCase.phpt b/ext/reflection/tests/ReflectionEnum_getCase.phpt
@@ -32,15 +32,15 @@ test(IntEnum::class, 'Baz');
 
 ?>
 --EXPECT--
-object(ReflectionEnumUnitCase)#2 (2) {
+object(ReflectionEnumUnitCase)#3 (2) {
   ["name"]=>
   string(3) "Foo"
   ["class"]=>
   string(5) "Enum_"
 }
 ReflectionException: Enum_::Bar is not a case
 ReflectionException: Case Enum_::Baz does not exist
-object(ReflectionEnumBackedCase)#2 (2) {
+object(ReflectionEnumBackedCase)#3 (2) {
   ["name"]=>
   string(3) "Foo"
   ["class"]=>
diff --git a/ext/reflection/tests/ReflectionEnum_getCases.phpt b/ext/reflection/tests/ReflectionEnum_getCases.phpt
