Skip to content

Add support for internal enums via stub generation #7212

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.

Sign up for GitHub

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

Add support for internal enums via stub generation #7212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion Zend/Optimizer/pass1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ void zend_optimizer_pass1(zend_op_array *op_array, zend_optimizer_ctx *ctx)
break;
}
}
if (Z_TYPE(c) == IS_CONSTANT_AST) {
if (Z_TYPE(c) == IS_CONSTANT_AST || Z_TYPE(c) == IS_OBJECT) {
break;
}
literal_dtor(&ZEND_OP2_LITERAL(opline));
Expand Down Expand Up @@ -274,6 +274,8 @@ void zend_optimizer_pass1(zend_op_array *op_array, zend_optimizer_ctx *ctx)
|| Z_TYPE(t) == IS_CONSTANT_AST) {
break;
}
} else if (Z_TYPE_P(c) == IS_OBJECT) {
break;
} else {
ZVAL_COPY_OR_DUP(&t, c);
}
Expand Down
16 changes: 16 additions & 0 deletions Zend/tests/enum/internal-enum-constant-assignment.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
--TEST--
Internal enum reassignment to constants preserves identity
--EXTENSIONS--
zend_test
--FILE--
<?php

const E = ZendTestStringBacked::FIRST;

var_dump(E);
var_dump(E === ZendTestStringBacked::FIRST);

?>
--EXPECT--
enum(ZendTestStringBacked::FIRST)
bool(true)
50 changes: 50 additions & 0 deletions Zend/tests/enum/internal-enum-persistent.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
--TEST--
Internal enums are persistent
--EXTENSIONS--
zend_test
--FILE--
<?php

var_dump(ZendTestStringBacked::FIRST);
var_dump(ZendTestStringBacked::FIRST == ZendTestStringBacked::getFirst());
var_dump(ZendTestStringBacked::FIRST === ZendTestStringBacked::getFirst());
var_dump(ZendTestStringBacked::FIRST !== ZendTestUnbacked::FIRST);
var_dump(ZendTestStringBacked::FIRST->value);
var_dump(ZendTestIntBacked::FIRST->value);

var_dump(ZendTestStringBacked::FIRST instanceof UnitEnum, ZendTestStringBacked::FIRST instanceof BackedEnum);
var_dump(ZendTestUnbacked::FIRST instanceof UnitEnum, ZendTestUnbacked::FIRST instanceof BackedEnum);

var_dump(ZendTestUnbacked::cases());
var_dump(ZendTestStringBacked::cases());
var_dump(ZendTestIntBacked::from(42));
var_dump(ZendTestStringBacked::tryFrom("foo"));
var_dump(ZendTestStringBacked::tryFrom("a"));

?>
--EXPECT--
enum(ZendTestStringBacked::FIRST)
bool(true)
bool(true)
bool(true)
string(1) "a"
int(42)
bool(true)
bool(true)
bool(true)
bool(false)
array(2) {
[0]=>
enum(ZendTestUnbacked::FIRST)
[1]=>
enum(ZendTestUnbacked::SECOND)
}
array(2) {
[0]=>
enum(ZendTestStringBacked::FIRST)
[1]=>
enum(ZendTestStringBacked::SECOND)
}
enum(ZendTestIntBacked::FIRST)
NULL
enum(ZendTestStringBacked::FIRST)
32 changes: 32 additions & 0 deletions Zend/tests/enum/internal-enum-read-from-reflection.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--TEST--
Internal enums can be safely read from reflection
--EXTENSIONS--
zend_test
--FILE--
<?php

$r = new ReflectionEnum("ZendTestStringBacked");
var_dump($r->getCases());
var_dump($r->getConstant("FIRST") === ZendTestStringBacked::FIRST);
var_dump($r->getConstants()["FIRST"] === ZendTestStringBacked::FIRST);

?>
--EXPECTF--
array(2) {
[0]=>
object(ReflectionEnumBackedCase)#%d (2) {
["name"]=>
string(5) "FIRST"
["class"]=>
string(20) "ZendTestStringBacked"
}
[1]=>
object(ReflectionEnumBackedCase)#%d (2) {
["name"]=>
string(6) "SECOND"
["class"]=>
string(20) "ZendTestStringBacked"
}
}
bool(true)
bool(true)
19 changes: 19 additions & 0 deletions Zend/tests/enum/internal-enum-serialization.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
--TEST--
Internal enums serialization
--EXTENSIONS--
zend_test
--FILE--
<?php

var_dump(ZendTestStringBacked::FIRST);
$s = serialize(ZendTestStringBacked::FIRST);
var_dump($s);
var_dump(unserialize($s));
var_dump(unserialize($s) === ZendTestStringBacked::FIRST);

?>
--EXPECT--
enum(ZendTestStringBacked::FIRST)
string(34) "E:26:"ZendTestStringBacked:FIRST";"
enum(ZendTestStringBacked::FIRST)
bool(true)
2 changes: 2 additions & 0 deletions Zend/zend_API.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ typedef struct _zend_function_entry {
uint32_t flags;
} zend_function_entry;

#include "zend_enum.h"

typedef struct _zend_fcall_info {
size_t size;
zval function_name;
Expand Down
148 changes: 139 additions & 9 deletions Zend/zend_enum.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
#include "zend.h"
#include "zend_API.h"
#include "zend_compile.h"
#include "zend_enum_arginfo.h"
#include "zend_interfaces.h"
#include "zend_inheritance.h"
#include "zend_constants.h"
#include "zend_enum_arginfo.h"

#define ZEND_ENUM_PROPERTY_ERROR() \
zend_throw_error(NULL, "Enum properties are immutable")
Expand All @@ -37,19 +39,22 @@ ZEND_API zend_class_entry *zend_ce_backed_enum;

static zend_object_handlers enum_handlers;

zend_object *zend_enum_new(zval *result, zend_class_entry *ce, zend_string *case_name, zval *backing_value_zv)
static void enum_obj_init(zend_object *obj, zend_string *case_name, zval *backing_value_zv)
{
zend_object *zobj = zend_objects_new(ce);
ZVAL_OBJ(result, zobj);

ZVAL_STR_COPY(OBJ_PROP_NUM(zobj, 0), case_name);
ZVAL_STR_COPY(OBJ_PROP_NUM(obj, 0), case_name);
if (backing_value_zv != NULL) {
ZVAL_COPY(OBJ_PROP_NUM(zobj, 1), backing_value_zv);
ZVAL_COPY(OBJ_PROP_NUM(obj, 1), backing_value_zv);
}

zobj->handlers = &enum_handlers;
obj->handlers = &enum_handlers;
}

return zobj;
zend_object *zend_enum_new(zval *result, zend_class_entry *ce, zend_string *case_name, zval *backing_value_zv)
{
zend_object *obj = zend_objects_new(ce);
ZVAL_OBJ(result, obj);
enum_obj_init(obj, case_name, backing_value_zv);
return obj;
}

static void zend_verify_enum_properties(zend_class_entry *ce)
Expand Down Expand Up @@ -371,3 +376,128 @@ void zend_enum_register_props(zend_class_entry *ce)
zend_declare_typed_property(ce, ZSTR_KNOWN(ZEND_STR_VALUE), &value_default_value, ZEND_ACC_PUBLIC, NULL, value_type);
}
}

static const zend_function_entry simple_internal_methods[] = {
ZEND_NAMED_ME(cases, zend_enum_cases_func, arginfo_class_UnitEnum_cases, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)
ZEND_FE_END
};

static const zend_function_entry backed_internal_methods[] = {
ZEND_NAMED_ME(from, zend_enum_from_func, arginfo_class_BackedEnum_from, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)
ZEND_NAMED_ME(tryFrom, zend_enum_try_from_func, arginfo_class_BackedEnum_tryFrom, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)
ZEND_FE_END
};

ZEND_API zend_class_entry *zend_register_internal_enum(const char *name)
{
return zend_register_internal_backed_enum_ex(name, IS_UNDEF, NULL);
}

ZEND_API zend_class_entry *zend_register_internal_enum_ex(const char *name, const zend_function_entry *functions)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd combine these two functions. Especially as these are intended for use with stubs, having an _ex variant to save a NULL doesn't seem particularly useful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I tend to agree, I should just distinguish backed vs unbacked.

{
return zend_register_internal_backed_enum_ex(name, IS_UNDEF, functions);
}

ZEND_API zend_class_entry *zend_register_internal_backed_enum(const char *name, uint32_t backing_type)
{
return zend_register_internal_backed_enum_ex(name, backing_type, NULL);
}

ZEND_API zend_class_entry *zend_register_internal_backed_enum_ex(const char *name, uint32_t backing_type, const zend_function_entry *functions)
{
ZEND_ASSERT(backing_type == IS_UNDEF || backing_type == IS_STRING || backing_type == IS_LONG);

zend_class_entry ce_val, *ce;
INIT_CLASS_ENTRY_EX(ce_val, name, strlen(name), functions);
ce = zend_register_internal_class(&ce_val);

ce->ce_flags |= ZEND_ACC_ENUM;
ce->enum_backing_type = backing_type;
ce->backed_enum_table = malloc(sizeof(HashTable));
zend_hash_init(ce->backed_enum_table, 0, NULL, ZVAL_PTR_DTOR, 1);
zend_enum_register_props(ce);

zend_register_functions(ce, simple_internal_methods, &ce->function_table, EG(current_module)->type);
zend_do_implement_interface(ce, zend_ce_unit_enum);
if (backing_type != IS_UNDEF) {
zend_register_functions(ce, backed_internal_methods, &ce->function_table, EG(current_module)->type);
zend_do_implement_interface(ce, zend_ce_backed_enum);
}

return ce;
}

static zend_object *add_internal_enum_case(zend_class_entry *ce, zend_string *name, zval *value)
{
zend_object *case_obj = zend_objects_new_persistent(ce);
enum_obj_init(case_obj, name, value);

zval casezv;
ZVAL_OBJ(&casezv, case_obj);
zend_class_constant *c = zend_declare_class_constant_ex(ce, name, &casezv, ZEND_ACC_PUBLIC, NULL);
ZEND_CLASS_CONST_FLAGS(c) |= ZEND_CLASS_CONST_IS_CASE;

zend_string_release(name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you releasing the name here? APIs generally shouldn't release values they are passed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel it's unergonomic to do zend_string * ... = zend_string_init(..); zend_register_enum_case(...); zend_string_release(...); it essentially becomes three statements instead of just one zend_register_enum_case(... zend_string_init (..)) … I particular I cannot imagine a case where you would want to retain the zend_string afterwards.

if (value) {
zval_internal_ptr_dtor(value);
}

return case_obj;
}

ZEND_API zend_object *zend_add_enum_case_ex(zend_class_entry *ce, zend_string *name)
{
ZEND_ASSERT(ce->enum_backing_type == IS_UNDEF);

name = zend_new_interned_string(name);

zend_object *case_obj = add_internal_enum_case(ce, name, NULL);

zend_string_release(name);

return case_obj;
}

ZEND_API zend_object *zend_add_enum_case_long_ex(zend_class_entry *ce, zend_string *name, zend_long value)
{
ZEND_ASSERT(ce->enum_backing_type == IS_LONG);

name = zend_new_interned_string(name);

zval namezv;
ZVAL_STR_COPY(&namezv, name);
if (!zend_hash_index_add(ce->backed_enum_table, value, &namezv)) {
ZEND_ASSERT(0 && "Cannot add two enum values with the same backing value");
}

zval zv;
ZVAL_LONG(&zv, value);
zend_object *case_obj = add_internal_enum_case(ce, name, &zv);

zend_string_release(name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments on ZVAL_INTERNED_STR and not needing zval_string_released on zend_new_interned_string. It seems like both zend_string_init_interned_permanent and zend_new_interned_string_request return interned strings where refcounting or freeing isn't necessary.

The code works but it's longer than it needs to be?


return case_obj;
}

ZEND_API zend_object *zend_add_enum_case_str_ex(zend_class_entry *ce, zend_string *name, zend_string *str)
{
ZEND_ASSERT(ce->enum_backing_type == IS_STRING);

name = zend_new_interned_string(name);
str = zend_new_interned_string(str);

zval namezv;
ZVAL_STR_COPY(&namezv, name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ZVAL_STR_COPY(&namezv, name);
ZVAL_INTERNED_STR(&namezv, name);

if (!zend_hash_add(ce->backed_enum_table, str, &namezv)) {
ZEND_ASSERT(0 && "Cannot add two enum values with the same backing value");
}

zval zv;
ZVAL_STR(&zv, str);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ZVAL_STR(&zv, str);
ZVAL_INTERNED_STR(&zv, str);

zend_object *case_obj = add_internal_enum_case(ce, name, &zv);

zend_string_release(name);
zend_string_release(str);
Comment on lines +499 to +500
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

static zend_always_inline void zend_string_release(zend_string *s)
{
	if (!ZSTR_IS_INTERNED(s)) {
		if (GC_DELREF(s) == 0) {
			pefree(s, GC_FLAGS(s) & IS_STR_PERSISTENT);
		}
	}
}
Suggested change
zend_string_release(name);
zend_string_release(str);


return case_obj;
}
17 changes: 17 additions & 0 deletions Zend/zend_enum.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define ZEND_ENUM_H

#include "zend.h"
#include "zend_API.h"
#include "zend_types.h"

BEGIN_EXTERN_C()
Expand Down Expand Up @@ -47,6 +48,22 @@ static zend_always_inline zval *zend_enum_fetch_case_value(zend_object *zobj)
return OBJ_PROP_NUM(zobj, 1);
}

ZEND_API zend_class_entry *zend_register_internal_enum(const char *name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these variations? I often find it the many functions hard to navigate. Maybe that's just me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends … well, you have to know that IS_UNDEF means unbacked and NULL means no functions (i.e. you don't need to define an empty array with just ZEND_FE_END). It's not always obvious what you would have to do if these simpler functions wouldn't be there.

Especially the distinction backed vs. unbacked makes sense to me

ZEND_API zend_class_entry *zend_register_internal_enum_ex(const char *name, const zend_function_entry *functions);
ZEND_API zend_class_entry *zend_register_internal_backed_enum(const char *name, uint32_t backing_type);
ZEND_API zend_class_entry *zend_register_internal_backed_enum_ex(const char *name, uint32_t backing_type, const zend_function_entry *functions);

#define zend_add_enum_case(ce, name) zend_add_enum_case_ex(ce, zend_string_init(name, sizeof(name) - 1, 1))
ZEND_API zend_object *zend_add_enum_case_ex(zend_class_entry *ce, zend_string *name);

#define zend_add_enum_case_long(ce, name, long) zend_add_enum_case_long_ex(ce, zend_string_init(name, sizeof(name) - 1, 1), long)
ZEND_API zend_object *zend_add_enum_case_long_ex(zend_class_entry *ce, zend_string *name, zend_long value);

#define zend_add_enum_case_string(ce, name, str) zend_add_enum_case_string_ex(ce, zend_string_init(name, sizeof(name) - 1, 1), str)
#define zend_add_enum_case_str(ce, name, str) zend_add_enum_case_str_ex(ce, zend_string_init(name, sizeof(name) - 1, 1), str)
#define zend_add_enum_case_string_ex(ce, name, str) zend_add_enum_case_str_ex(ce, name, zend_string_init(str, sizeof(str) - 1, 1))
ZEND_API zend_object *zend_add_enum_case_str_ex(zend_class_entry *ce, zend_string *name, zend_string *str);

END_EXTERN_C()

#endif /* ZEND_ENUM_H */
2 changes: 2 additions & 0 deletions Zend/zend_execute_API.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ void init_executor(void) /* {{{ */
EG(persistent_constants_count) = EG(zend_constants)->nNumUsed;
EG(persistent_functions_count) = EG(function_table)->nNumUsed;
EG(persistent_classes_count) = EG(class_table)->nNumUsed;
EG(persistent_objects) = (zend_object *) ecalloc(sizeof(zend_object), zend_objects_persistent_handle_counter);

EG(get_gc_buffer).start = EG(get_gc_buffer).end = EG(get_gc_buffer).cur = NULL;

Expand Down Expand Up @@ -431,6 +432,7 @@ void shutdown_executor(void) /* {{{ */
zend_stack_destroy(&EG(user_error_handlers_error_reporting));
zend_stack_destroy(&EG(user_error_handlers));
zend_stack_destroy(&EG(user_exception_handlers));
efree(EG(persistent_objects));
zend_objects_store_destroy(&EG(objects_store));
if (EG(in_autoload)) {
zend_hash_destroy(EG(in_autoload));
Expand Down
10 changes: 5 additions & 5 deletions Zend/zend_gc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,13 @@
#endif

/* GC_INFO layout */
#define GC_ADDRESS 0x0fffffu
#define GC_COLOR 0x300000u
#define GC_ADDRESS 0x07ffffu
#define GC_COLOR 0x180000u

#define GC_BLACK 0x000000u /* must be zero */
#define GC_WHITE 0x100000u
#define GC_GREY 0x200000u
#define GC_PURPLE 0x300000u
#define GC_WHITE 0x080000u
#define GC_GREY 0x100000u
#define GC_PURPLE 0x180000u

/* Debug tracing */
#if ZEND_GC_DEBUG > 1
Expand Down
1 change: 1 addition & 0 deletions Zend/zend_globals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ struct _zend_executor_globals {
uint32_t persistent_constants_count;
uint32_t persistent_functions_count;
uint32_t persistent_classes_count;
zend_object *persistent_objects;

HashTable *in_autoload;
bool full_tables_cleanup;
Expand Down
Loading