Skip to content

Commit

Permalink
Implemented the RFC Support Class Constant Visibility.
Browse files Browse the repository at this point in the history 
Squashed commit of the following:

commit f11ca0e
Author: Dmitry Stogov <dmitry@zend.com>
Date:   Tue Dec 8 12:38:42 2015 +0300

    Fixed test expectation

commit 211f873
Author: Dmitry Stogov <dmitry@zend.com>
Date:   Tue Dec 8 12:28:38 2015 +0300

    Embed zend_class_constant.flags into zend_class_constants.value.u2.access_flags

commit 51deab8
Author: Dmitry Stogov <dmitry@zend.com>
Date:   Mon Dec 7 11:18:55 2015 +0300

    Fixed issues found by Nikita

commit 544dbd5
Author: Dmitry Stogov <dmitry@zend.com>
Date:   Sat Dec 5 02:41:05 2015 +0300

    Refactored immplementation of https://wiki.php.net/rfc/class_const_visibility
    @reeze created an RFC here and I emailed internals here and didn't get any responses positive/negative.
  • Loading branch information
@dstogov
dstogov committed Dec 8, 2015
1 parent a8b7d0c commit a75c195
Show file tree
Hide file tree
Showing 40 changed files with 1,143 additions and 126 deletions.
42 changes: 38 additions & 4 deletions Zend/zend_API.c
View file
Original file line number Original file line 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 **scope = EG(current_execute_data) ? &EG(scope) : &CG(active_class_entry);
zend_class_entry *old_scope = *scope; zend_class_entry *old_scope = *scope;
zend_class_entry *ce; zend_class_entry *ce;
zend_class_constant *c;
zval *val; zval *val;
zend_property_info *prop_info; zend_property_info *prop_info;


*scope = class_type; *scope = class_type;
ZEND_HASH_FOREACH_VAL(&class_type->constants_table, val) { ZEND_HASH_FOREACH_PTR(&class_type->constants_table, c) {
ZVAL_DEREF(val); val = &c->value;
if (Z_CONSTANT_P(val)) { if (Z_CONSTANT_P(val)) {
if (UNEXPECTED(zval_update_constant_ex(val, 1, class_type) != SUCCESS)) { if (UNEXPECTED(zval_update_constant_ex(val, 1, class_type) != SUCCESS)) {
return FAILURE; 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)) { if (Z_CONSTANT_P(value)) {
ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED; 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; 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) /* {{{ */ ZEND_API int zend_declare_class_constant_null(zend_class_entry *ce, const char *name, size_t name_length) /* {{{ */
{ {
zval constant; zval constant;
Expand Down
1 change: 1 addition & 0 deletions Zend/zend_API.h
View file
Original file line number Original file line 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_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_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(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_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); 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
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ enum _zend_ast_kind {
ZEND_AST_SWITCH, ZEND_AST_SWITCH,
ZEND_AST_SWITCH_CASE, ZEND_AST_SWITCH_CASE,
ZEND_AST_DECLARE, ZEND_AST_DECLARE,
ZEND_AST_CONST_ELEM,
ZEND_AST_USE_TRAIT, ZEND_AST_USE_TRAIT,
ZEND_AST_TRAIT_PRECEDENCE, ZEND_AST_TRAIT_PRECEDENCE,
ZEND_AST_METHOD_REFERENCE, ZEND_AST_METHOD_REFERENCE,
Expand All @@ -142,6 +141,7 @@ enum _zend_ast_kind {
ZEND_AST_CATCH, ZEND_AST_CATCH,
ZEND_AST_PARAM, ZEND_AST_PARAM,
ZEND_AST_PROP_ELEM, ZEND_AST_PROP_ELEM,
ZEND_AST_CONST_ELEM,


/* 4 child nodes */ /* 4 child nodes */
ZEND_AST_FOR = 4 << ZEND_AST_NUM_CHILDREN_SHIFT, ZEND_AST_FOR = 4 << ZEND_AST_NUM_CHILDREN_SHIFT,
Expand Down
41 changes: 28 additions & 13 deletions Zend/zend_compile.c
View file
Original file line number Original file line 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) /* {{{ */ { static zend_string *zend_new_interned_string_safe(zend_string *str) /* {{{ */ {
zend_string *interned_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) /* {{{ */ 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); uint32_t fetch_type = zend_get_class_fetch_type(class_name);
zend_class_constant *cc;
zval *c; zval *c;


if (class_name_refers_to_active_ce(class_name, fetch_type)) { 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)) { } 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)); zend_class_entry *ce = zend_hash_find_ptr_lc(CG(class_table), ZSTR_VAL(class_name), ZSTR_LEN(class_name));
if (ce) { if (ce) {
c = zend_hash_find(&ce->constants_table, name); cc = zend_hash_find_ptr(&ce->constants_table, name);
} else { } else {
return 0; 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; 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 */ /* 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); ZVAL_DUP(zv, c);
return 1; 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_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; 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->refcount = 1;
ce->ce_flags = ZEND_ACC_CONSTANTS_UPDATED; 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_properties_table = NULL;
ce->default_static_members_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->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); zend_hash_init_ex(&ce->function_table, 8, NULL, ZEND_FUNCTION_DTOR, persistent_hashes, 0);


if (ce->type == ZEND_INTERNAL_CLASS) { 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 *const_ast = list->child[i];
zend_ast *name_ast = const_ast->child[0]; zend_ast *name_ast = const_ast->child[0];
zend_ast *value_ast = const_ast->child[1]; 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 *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; zval value_zv;


if (zend_string_equals_literal_ci(name, "class")) { if (UNEXPECTED(ast->attr & (ZEND_ACC_STATIC|ZEND_ACC_ABSTRACT|ZEND_ACC_FINAL))) {
zend_error(E_COMPILE_ERROR, if (ast->attr & ZEND_ACC_STATIC) {
"A class constant must not be called 'class'; it is reserved for class name fetching"); 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); zend_const_expr_to_zval(&value_zv, value_ast);


name = zend_new_interned_string_safe(name); 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", zend_error_noreturn(E_COMPILE_ERROR, "Cannot redefine class constant %s::%s",
ZSTR_VAL(ce->name), ZSTR_VAL(name)); 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
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -310,6 +310,12 @@ typedef struct _zend_property_info {
#define OBJ_PROP_TO_NUM(offset) \ #define OBJ_PROP_TO_NUM(offset) \
((offset - OBJ_PROP_TO_OFFSET(0)) / sizeof(zval)) ((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 */ /* arg_info for internal functions */
typedef struct _zend_internal_arg_info { typedef struct _zend_internal_arg_info {
const char *name; const char *name;
Expand Down
27 changes: 23 additions & 4 deletions Zend/zend_constants.c
View file
Original file line number Original file line 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) 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); free_alloca(lcname, use_heap);
if (ce) { if (ce) {
ret_constant = zend_hash_find(&ce->constants_table, constant_name); zend_class_constant *c = zend_hash_find_ptr(&ce->constants_table, constant_name);
if (ret_constant == NULL) { if (c == NULL) {
if ((flags & ZEND_FETCH_CLASS_SILENT) == 0) { 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_throw_error(NULL, "Undefined class constant '%s::%s'", ZSTR_VAL(class_name), ZSTR_VAL(constant_name));
zend_string_release(class_name); zend_string_release(class_name);
zend_string_free(constant_name); zend_string_free(constant_name);
return NULL; return NULL;
} }
} else if (Z_ISREF_P(ret_constant)) { ret_constant = NULL;
ret_constant = Z_REFVAL_P(ret_constant); } 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); zend_string_release(class_name);
Expand Down
1 change: 1 addition & 0 deletions Zend/zend_constants.h
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ int zend_startup_constants(void);
int zend_shutdown_constants(void); int zend_shutdown_constants(void);
void zend_register_standard_constants(void); void zend_register_standard_constants(void);
void clean_non_persistent_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(zend_string *name);
ZEND_API zval *zend_get_constant_str(const char *name, size_t name_len); 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); ZEND_API zval *zend_get_constant_ex(zend_string *name, zend_class_entry *scope, uint32_t flags);
Expand Down
Loading

2 comments on commit a75c195

@Majkl578
Copy link
Contributor

@Majkl578 Majkl578 commented on a75c195 May 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Hi, while playing with this (and other) new feature(s), I've noticed a bug in ReflectionClassConstant::getValue(). When constant contains an expression referencing another constant, the result is incorrect.

class Foo {
    const X = self::Y * 2;
    const Y = 1;
}

//var_dump((new ReflectionClass(Foo::class))->getConstants());
var_dump((new ReflectionClass(Foo::class))->getReflectionConstant('X')->getValue()); // UNKNOWN:0

The output of this code is UNKNOWN:0. If you swap the order of X and Y (so that Y is defined sooner), value is correct. Uncommenting the call to getConstants() also fixes the issue.
(I'll report it on bugs.php.net eventually, later when I have more time.)

@nikic
Copy link
Member

@nikic nikic commented on a75c195 May 2, 2016

Choose a reason for hiding this comment

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

@Majkl578 Yeah, there's a couple of "update constant" operations missing in the new code. I'm on it.

Please sign in to comment.