Skip to content

Refactored immplementation of https://wiki.php.net/rfc/class_const_vi… #1662

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 4 commits into from
Closed

Refactored immplementation of https://wiki.php.net/rfc/class_const_vi… #1662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
544dbd5
Refactored immplementation of https://wiki.php.net/rfc/class_const_vi…
dstogov Dec 4, 2015
51deab8
Fixed issues found by Nikita
dstogov Dec 7, 2015
211f873
Embed zend_class_constant.flags into zend_class_constants.value.u2.ac…
dstogov Dec 8, 2015
f11ca0e
Fixed test expectation
dstogov Dec 8, 2015
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
42 changes: 38 additions & 4 deletions Zend/zend_API.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1129,12 +1129,13 @@ ZEND_API int zend_update_class_constants(zend_class_entry *class_type) /* {{{ */
zend_class_entry **scope = EG(current_execute_data) ? &EG(scope) : &CG(active_class_entry);
zend_class_entry *old_scope = *scope;
zend_class_entry *ce;
zend_class_constant *c;
zval *val;
zend_property_info *prop_info;

*scope = class_type;
ZEND_HASH_FOREACH_VAL(&class_type->constants_table, val) {
ZVAL_DEREF(val);
ZEND_HASH_FOREACH_PTR(&class_type->constants_table, c) {
val = &c->value;
if (Z_CONSTANT_P(val)) {
if (UNEXPECTED(zval_update_constant_ex(val, 1, class_type) != SUCCESS)) {
return FAILURE;
Expand Down Expand Up @@ -3737,16 +3738,49 @@ ZEND_API int zend_declare_property_stringl(zend_class_entry *ce, const char *nam
}
/* }}} */

ZEND_API int zend_declare_class_constant(zend_class_entry *ce, const char *name, size_t name_length, zval *value) /* {{{ */
ZEND_API int zend_declare_class_constant_ex(zend_class_entry *ce, zend_string *name, zval *value, int access_type, zend_string *doc_comment) /* {{{ */
{
zend_class_constant *c;

if (ce->ce_flags & ZEND_ACC_INTERFACE) {
if (access_type != ZEND_ACC_PUBLIC) {
zend_error_noreturn(E_COMPILE_ERROR, "Access type for interface constant %s::%s must be public", ZSTR_VAL(ce->name), ZSTR_VAL(name));
}
}

if (zend_string_equals_literal_ci(name, "class")) {
zend_error((ce->type == ZEND_INTERNAL_CLASS) ? E_CORE_ERROR : E_COMPILE_ERROR,
"A class constant must not be called 'class'; it is reserved for class name fetching");
}

if (ce->type == ZEND_INTERNAL_CLASS) {
c = pemalloc(sizeof(zend_class_constant), 1);
} else {
c = zend_arena_alloc(&CG(arena), sizeof(zend_class_constant));
}
ZVAL_COPY_VALUE(&c->value, value);
Z_ACCESS_FLAGS(c->value) = access_type;
c->doc_comment = doc_comment;
c->ce = ce;
if (Z_CONSTANT_P(value)) {
ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED;
}
return zend_hash_str_update(&ce->constants_table, name, name_length, value) ?
return zend_hash_add_ptr(&ce->constants_table, name, c) ?
SUCCESS : FAILURE;
}
/* }}} */

ZEND_API int zend_declare_class_constant(zend_class_entry *ce, const char *name, size_t name_length, zval *value) /* {{{ */
{
int ret;

zend_string *key = zend_string_init(name, name_length, ce->type & ZEND_INTERNAL_CLASS);
ret = zend_declare_class_constant_ex(ce, key, value, ZEND_ACC_PUBLIC, NULL);
zend_string_release(key);
return ret;
}
/* }}} */

ZEND_API int zend_declare_class_constant_null(zend_class_entry *ce, const char *name, size_t name_length) /* {{{ */
{
zval constant;
Expand Down
1 change: 1 addition & 0 deletions Zend/zend_API.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ ZEND_API int zend_declare_property_double(zend_class_entry *ce, const char *name
ZEND_API int zend_declare_property_string(zend_class_entry *ce, const char *name, size_t name_length, const char *value, int access_type);
ZEND_API int zend_declare_property_stringl(zend_class_entry *ce, const char *name, size_t name_length, const char *value, size_t value_len, int access_type);

ZEND_API int zend_declare_class_constant_ex(zend_class_entry *ce, zend_string *name, zval *value, int access_type, zend_string *doc_comment);
ZEND_API int zend_declare_class_constant(zend_class_entry *ce, const char *name, size_t name_length, zval *value);
ZEND_API int zend_declare_class_constant_null(zend_class_entry *ce, const char *name, size_t name_length);
ZEND_API int zend_declare_class_constant_long(zend_class_entry *ce, const char *name, size_t name_length, zend_long value);
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_ast.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ enum _zend_ast_kind {
ZEND_AST_SWITCH,
ZEND_AST_SWITCH_CASE,
ZEND_AST_DECLARE,
ZEND_AST_CONST_ELEM,
ZEND_AST_USE_TRAIT,
ZEND_AST_TRAIT_PRECEDENCE,
ZEND_AST_METHOD_REFERENCE,
Expand All @@ -142,6 +141,7 @@ enum _zend_ast_kind {
ZEND_AST_CATCH,
ZEND_AST_PARAM,
ZEND_AST_PROP_ELEM,
ZEND_AST_CONST_ELEM,

/* 4 child nodes */
ZEND_AST_FOR = 4 << ZEND_AST_NUM_CHILDREN_SHIFT,
Expand Down
41 changes: 28 additions & 13 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ static void zend_destroy_property_info_internal(zval *zv) /* {{{ */
}
/* }}} */

static void zend_destroy_class_constant_internal(zval *zv) /* {{{ */
{
free(Z_PTR_P(zv));
}
/* }}} */

static zend_string *zend_new_interned_string_safe(zend_string *str) /* {{{ */ {
zend_string *interned_str;

Expand Down Expand Up @@ -1437,14 +1443,15 @@ static zend_bool zend_try_compile_const_expr_resolve_class_name(zval *zv, zend_a
static zend_bool zend_try_ct_eval_class_const(zval *zv, zend_string *class_name, zend_string *name) /* {{{ */
{
uint32_t fetch_type = zend_get_class_fetch_type(class_name);
zend_class_constant *cc;
zval *c;

if (class_name_refers_to_active_ce(class_name, fetch_type)) {
c = zend_hash_find(&CG(active_class_entry)->constants_table, name);
cc = zend_hash_find_ptr(&CG(active_class_entry)->constants_table, name);
} else if (fetch_type == ZEND_FETCH_CLASS_DEFAULT && !(CG(compiler_options) & ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION)) {
zend_class_entry *ce = zend_hash_find_ptr_lc(CG(class_table), ZSTR_VAL(class_name), ZSTR_LEN(class_name));
if (ce) {
c = zend_hash_find(&ce->constants_table, name);
cc = zend_hash_find_ptr(&ce->constants_table, name);
} else {
return 0;
}
Expand All @@ -1456,8 +1463,14 @@ static zend_bool zend_try_ct_eval_class_const(zval *zv, zend_string *class_name,
return 0;
}

if (!cc || !zend_verify_const_access(cc, CG(active_class_entry))) {
return 0;
}

c = &cc->value;

/* Substitute case-sensitive (or lowercase) persistent class constants */
if (c && Z_TYPE_P(c) < IS_OBJECT) {
if (Z_TYPE_P(c) < IS_OBJECT) {
ZVAL_DUP(zv, c);
Copy link
Member

Choose a reason for hiding this comment

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

I know it's not introduced by this patch, but why ZVAL_DUP here? I think ZVAL_COPY here should be enough

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 also think ZVAL_COPY() should be enough, but lets change this in a
separate patch.

On Mon, Dec 7, 2015 at 7:27 PM, Xinchen Hui [email protected]
wrote:

In Zend/zend_compile.c
#1662 (comment):

/* Substitute case-sensitive (or lowercase) persistent class constants */
  • if (c && Z_TYPE_P(c) < IS_OBJECT) {
  • if (Z_TYPE_P(c) < IS_OBJECT) {
    ZVAL_DUP(zv, c);

I know it's not introduced by this patch, but why ZVAL_DUP here? I think
ZVAL_COPY here should be enough

—
Reply to this email directly or view it on GitHub
https://github.com/php/php-src/pull/1662/files#r46841212.

return 1;
}
Expand Down Expand Up @@ -1638,7 +1651,6 @@ int zendlex(zend_parser_stack_elem *elem) /* {{{ */
ZEND_API void zend_initialize_class_data(zend_class_entry *ce, zend_bool nullify_handlers) /* {{{ */
{
zend_bool persistent_hashes = (ce->type == ZEND_INTERNAL_CLASS) ? 1 : 0;
dtor_func_t zval_ptr_dtor_func = ((persistent_hashes) ? ZVAL_INTERNAL_PTR_DTOR : ZVAL_PTR_DTOR);

ce->refcount = 1;
ce->ce_flags = ZEND_ACC_CONSTANTS_UPDATED;
Expand All @@ -1650,7 +1662,7 @@ ZEND_API void zend_initialize_class_data(zend_class_entry *ce, zend_bool nullify
ce->default_properties_table = NULL;
ce->default_static_members_table = NULL;
zend_hash_init_ex(&ce->properties_info, 8, NULL, (persistent_hashes ? zend_destroy_property_info_internal : NULL), persistent_hashes, 0);
zend_hash_init_ex(&ce->constants_table, 8, NULL, zval_ptr_dtor_func, persistent_hashes, 0);
zend_hash_init_ex(&ce->constants_table, 8, NULL, (persistent_hashes ? zend_destroy_class_constant_internal : NULL), persistent_hashes, 0);
zend_hash_init_ex(&ce->function_table, 8, NULL, ZEND_FUNCTION_DTOR, persistent_hashes, 0);

if (ce->type == ZEND_INTERNAL_CLASS) {
Expand Down Expand Up @@ -5183,25 +5195,28 @@ void zend_compile_class_const_decl(zend_ast *ast) /* {{{ */
zend_ast *const_ast = list->child[i];
zend_ast *name_ast = const_ast->child[0];
zend_ast *value_ast = const_ast->child[1];
zend_ast *doc_comment_ast = const_ast->child[2];
zend_string *name = zend_ast_get_str(name_ast);
zend_string *doc_comment = doc_comment_ast ? zend_string_copy(zend_ast_get_str(doc_comment_ast)) : NULL;
zval value_zv;

if (zend_string_equals_literal_ci(name, "class")) {
zend_error(E_COMPILE_ERROR,
"A class constant must not be called 'class'; it is reserved for class name fetching");
if (UNEXPECTED(ast->attr & (ZEND_ACC_STATIC|ZEND_ACC_ABSTRACT|ZEND_ACC_FINAL))) {
if (ast->attr & ZEND_ACC_STATIC) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'static' as constant modifier");
} else if (ast->attr & ZEND_ACC_ABSTRACT) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'abstract' as constant modifier");
} else if (ast->attr & ZEND_ACC_FINAL) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot use 'final' as constant modifier");
}
}

zend_const_expr_to_zval(&value_zv, value_ast);

name = zend_new_interned_string_safe(name);
if (zend_hash_add(&ce->constants_table, name, &value_zv) == NULL) {
if (zend_declare_class_constant_ex(ce, name, &value_zv, ast->attr, doc_comment) != SUCCESS) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot redefine class constant %s::%s",
ZSTR_VAL(ce->name), ZSTR_VAL(name));
}

if (Z_CONSTANT(value_zv)) {
ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED;
}
}
}
/* }}} */
Expand Down
6 changes: 6 additions & 0 deletions Zend/zend_compile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,12 @@ typedef struct _zend_property_info {
#define OBJ_PROP_TO_NUM(offset) \
((offset - OBJ_PROP_TO_OFFSET(0)) / sizeof(zval))

typedef struct _zend_class_constant {
zval value; /* access flags are stored in reserved: zval.u2.access_flags */
zend_string *doc_comment;
zend_class_entry *ce;
} zend_class_constant;

/* arg_info for internal functions */
typedef struct _zend_internal_arg_info {
const char *name;
Expand Down
27 changes: 23 additions & 4 deletions Zend/zend_constants.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,18 @@ static zend_constant *zend_get_special_constant(const char *name, size_t name_le
}
}

ZEND_API int zend_verify_const_access(zend_class_constant *c, zend_class_entry *scope) /* {{{ */
{
if (Z_ACCESS_FLAGS(c->value) & ZEND_ACC_PUBLIC) {
return 1;
} else if (Z_ACCESS_FLAGS(c->value) & ZEND_ACC_PRIVATE) {
return (c->ce == scope);
} else {
ZEND_ASSERT(Z_ACCESS_FLAGS(c->value) & ZEND_ACC_PROTECTED);
return zend_check_protected(c->ce, scope);
}
}
/* }}} */

ZEND_API zval *zend_get_constant_str(const char *name, size_t name_len)
{
Expand Down Expand Up @@ -360,16 +372,23 @@ ZEND_API zval *zend_get_constant_ex(zend_string *cname, zend_class_entry *scope,
}
free_alloca(lcname, use_heap);
if (ce) {
ret_constant = zend_hash_find(&ce->constants_table, constant_name);
if (ret_constant == NULL) {
zend_class_constant *c = zend_hash_find_ptr(&ce->constants_table, constant_name);
if (c == NULL) {
if ((flags & ZEND_FETCH_CLASS_SILENT) == 0) {
zend_throw_error(NULL, "Undefined class constant '%s::%s'", ZSTR_VAL(class_name), ZSTR_VAL(constant_name));
zend_string_release(class_name);
zend_string_free(constant_name);
return NULL;
}
} else if (Z_ISREF_P(ret_constant)) {
ret_constant = Z_REFVAL_P(ret_constant);
ret_constant = NULL;
} else {
if (!zend_verify_const_access(c, scope)) {
zend_throw_error(NULL, "Cannot access %s const %s::%s", zend_visibility_string(Z_ACCESS_FLAGS(c->value)), ZSTR_VAL(class_name), ZSTR_VAL(constant_name));
zend_string_release(class_name);
zend_string_free(constant_name);
return NULL;
}
ret_constant = &c->value;
}
}
zend_string_release(class_name);
Expand Down
1 change: 1 addition & 0 deletions Zend/zend_constants.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ int zend_startup_constants(void);
int zend_shutdown_constants(void);
void zend_register_standard_constants(void);
void clean_non_persistent_constants(void);
ZEND_API int zend_verify_const_access(zend_class_constant *c, zend_class_entry *ce);
ZEND_API zval *zend_get_constant(zend_string *name);
ZEND_API zval *zend_get_constant_str(const char *name, size_t name_len);
ZEND_API zval *zend_get_constant_ex(zend_string *name, zend_class_entry *scope, uint32_t flags);
Expand Down
Loading