Skip to content

Allow arbitrary const expressions in backed enums #8190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions Zend/tests/enum/backed-duplicate-int.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,33 @@ enum Foo: int {
case Baz = 0;
}

try {
var_dump(Foo::Bar);
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

try {
var_dump(Foo::Bar);
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

try {
var_dump(Foo::from(42));
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

try {
var_dump(Foo::tryFrom('bar'));
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

?>
--EXPECTF--
Fatal error: Duplicate value in enum Foo for cases Bar and Baz in %s on line %s
--EXPECT--
Duplicate value in enum Foo for cases Bar and Baz
Duplicate value in enum Foo for cases Bar and Baz
Duplicate value in enum Foo for cases Bar and Baz
Foo::tryFrom(): Argument #1 ($value) must be of type int, string given
31 changes: 29 additions & 2 deletions Zend/tests/enum/backed-duplicate-string.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,33 @@ enum Suit: string {
case Spades = 'H';
}

try {
var_dump(Suit::Hearts);
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

try {
var_dump(Suit::Hearts);
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

try {
var_dump(Suit::from(42));
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

try {
var_dump(Suit::tryFrom('bar'));
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

?>
--EXPECTF--
Fatal error: Duplicate value in enum Suit for cases Hearts and Spades in %s on line %s
--EXPECT--
Duplicate value in enum Suit for cases Hearts and Spades
Duplicate value in enum Suit for cases Hearts and Spades
Duplicate value in enum Suit for cases Hearts and Spades
Duplicate value in enum Suit for cases Hearts and Spades
2 changes: 1 addition & 1 deletion Zend/tests/enum/backed-int-const-invalid-expr.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
31 changes: 29 additions & 2 deletions Zend/tests/enum/backed-mismatch.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,33 @@ enum Foo: int {
case Bar = 'bar';
}

try {
var_dump(Foo::Bar);
} catch (Error $e) {
echo get_class($e), ': ', $e->getMessage(), "\n";
}

try {
var_dump(Foo::Bar);
} catch (Error $e) {
echo get_class($e), ': ', $e->getMessage(), "\n";
}

try {
var_dump(Foo::from(42));
} catch (Error $e) {
echo get_class($e), ': ', $e->getMessage(), "\n";
}

try {
var_dump(Foo::from('bar'));
} catch (Error $e) {
echo get_class($e), ': ', $e->getMessage(), "\n";
}

?>
--EXPECTF--
Fatal error: Enum case type string does not match enum backing type int in %s on line %d
--EXPECT--
TypeError: Enum case type string does not match enum backing type int
TypeError: Enum case type string does not match enum backing type int
TypeError: Enum case type string does not match enum backing type int
TypeError: Foo::from(): Argument #1 ($value) must be of type int, string given
26 changes: 26 additions & 0 deletions Zend/tests/enum/gh7821.phpt
Original file line number Diff line number Diff line change
@@ -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"
20 changes: 20 additions & 0 deletions Zend/tests/enum/gh8418.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--TEST--
GH-8418: Enum constant expression evaluation doesn't work with warmed cache
--FILE--
<?php

class ExampleClass
{
public const EXAMPLE_CONST = 42;
}

enum ExampleEnum: int
{
case ENUM_CASE = ExampleClass::EXAMPLE_CONST;
}

var_dump(ExampleEnum::ENUM_CASE->value);

?>
--EXPECT--
int(42)
2 changes: 1 addition & 1 deletion Zend/tests/enum/non-backed-enum-with-expr-value.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion Zend/tests/enum/non-backed-enum-with-int-value.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion Zend/tests/enum/non-backed-enum-with-string-value.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 17 additions & 0 deletions Zend/tests/enum/update-class-constant-failure.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
--TEST--
Test failure of updating class constants
--FILE--
<?php

enum Foo: string {
const Bar = NONEXISTENT;
}

var_dump(Foo::Bar);

?>
--EXPECTF--
Fatal error: Uncaught Error: Undefined constant "NONEXISTENT" in %s:%d
Stack trace:
#0 {main}
thrown in %s on line %d
7 changes: 7 additions & 0 deletions Zend/zend_API.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "zend_closures.h"
#include "zend_inheritance.h"
#include "zend_ini.h"
#include "zend_enum.h"

#include <stdarg.h>

Expand Down Expand Up @@ -1493,6 +1494,12 @@ ZEND_API zend_result zend_update_class_constants(zend_class_entry *class_type) /
}
}

if (class_type->type == ZEND_USER_CLASS && class_type->ce_flags & ZEND_ACC_ENUM && class_type->enum_backing_type != IS_UNDEF) {
if (zend_enum_build_backed_enum_table(class_type) == FAILURE) {
return FAILURE;
}
}

ce_flags |= ZEND_ACC_CONSTANTS_UPDATED;
ce_flags &= ~ZEND_ACC_HAS_AST_CONSTANTS;
ce_flags &= ~ZEND_ACC_HAS_AST_PROPERTIES;
Expand Down
14 changes: 10 additions & 4 deletions Zend/zend_ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -776,12 +776,18 @@ 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_UNDEF(&case_value_zv);
if (case_value_ast != NULL) {
if (UNEXPECTED(zend_ast_evaluate(&case_value_zv, case_value_ast, scope) != SUCCESS)) {
return FAILURE;
}
}

zend_class_entry *ce = zend_lookup_class(class_name);
zend_enum_new(result, ce, case_name, case_value_zv);
zend_enum_new(result, ce, case_name, case_value_ast != NULL ? &case_value_zv : NULL);
zval_ptr_dtor_nogc(&case_value_zv);
break;
}
case ZEND_AST_CLASS_CONST:
Expand Down
66 changes: 7 additions & 59 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -7575,9 +7575,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) /* {{{ */
Expand Down Expand Up @@ -7789,69 +7786,20 @@ 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];
// Remove case_value_ast from the original AST to avoid freeing it, as it will be freed by zend_const_expr_to_zval
ast->child[1] = NULL;
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);
Expand Down
Loading