Skip to content
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

Reflection: show the type of object constants used as default properties #15922

Closed
wants to merge 2 commits 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
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ PHP NEWS
- PHPDBG:
. Fixed bug GH-16174 (Empty string is an invalid expression for ev). (cmb)

- Reflection:
. Fixed bug GH-15902 (Core dumped in ext/reflection/php_reflection.c).
(DanielEScherzer)

- Session:
. Fixed bug GH-16385 (Unexpected null returned by session_set_cookie_params).
(nielsdos)
Expand Down
21 changes: 15 additions & 6 deletions ext/reflection/php_reflection.c
Original file line number Diff line number Diff line change
Expand Up @@ -639,13 +639,19 @@ static int format_default_value(smart_str *str, zval *value) {
} ZEND_HASH_FOREACH_END();
smart_str_appendc(str, ']');
} else if (Z_TYPE_P(value) == IS_OBJECT) {
/* This branch may only be reached for default properties, which don't support arbitrary objects. */
/* This branch may only be reached for default properties; show enum names,
or the type of non-enums (GH-15902) */
zend_object *obj = Z_OBJ_P(value);
zend_class_entry *class = obj->ce;
ZEND_ASSERT(class->ce_flags & ZEND_ACC_ENUM);
smart_str_append(str, class->name);
smart_str_appends(str, "::");
smart_str_append(str, Z_STR_P(zend_enum_fetch_case_name(obj)));
if (class->ce_flags & ZEND_ACC_ENUM) {
smart_str_append(str, class->name);
smart_str_appends(str, "::");
smart_str_append(str, Z_STR_P(zend_enum_fetch_case_name(obj)));
} else {
smart_str_appends(str, "object(");
smart_str_append(str, class->name);
smart_str_appends(str, ")");
}
} else {
ZEND_ASSERT(Z_TYPE_P(value) == IS_CONSTANT_AST);
zend_string *ast_str = zend_ast_export("", Z_ASTVAL_P(value), "");
Expand Down Expand Up @@ -878,7 +884,10 @@ static zval *property_get_default(zend_property_info *prop_info) {
ZVAL_DEINDIRECT(prop);
return prop;
} else {
return &ce->default_properties_table[OBJ_PROP_TO_NUM(prop_info->offset)];
if (UNEXPECTED(zend_update_class_constants(ce) != SUCCESS)) {
return &EG(uninitialized_zval);
}
return &CE_DEFAULT_PROPERTIES_TABLE(ce)[OBJ_PROP_TO_NUM(prop_info->offset)];
Copy link
Member

Choose a reason for hiding this comment

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

Note though that this is actually kind of breaking. We're changing the default value to show the evaluated constant, rather than the constant AST. This was already the case when not using opcache, because we're evaluating the AST in-place. However, when using opcache, the AST is locked in shm and copied into "userspace" to be evaluated there. Adjusting this makes sense IMO to match the behavior of opcache and non-opcache, but I think that's better fixed on master.

Maybe you can propose this in a separate PR on master, and adjust tests to disable opcache (opcache.enable_cli=0 in --INI--).

}
}

Expand Down
36 changes: 36 additions & 0 deletions ext/reflection/tests/gh15902/ReflectionClass-callable.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
--TEST--
ReflectionClass object default property - used to say "callable"
--FILE--
<?php

class C {
public stdClass $a = FOO;
}
define('FOO', new stdClass);

new C;

$reflector = new ReflectionClass(C::class);
var_dump( (string)$reflector );
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the spaces inside parens

Copy link
Member

Choose a reason for hiding this comment

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

Also, this can just use echo $reflector;, the var_dump doesn't really have a purpose.

?>
--EXPECTF--
string(%d) "Class [ <user> class C ] {
@@ %sReflectionClass-callable.php %d-%d

- Constants [0] {
}

- Static properties [0] {
}

- Static methods [0] {
}

- Properties [1] {
Property [ public stdClass $a = object(stdClass) ]
}

- Methods [0] {
}
}
"
37 changes: 37 additions & 0 deletions ext/reflection/tests/gh15902/ReflectionClass-class.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
--TEST--
ReflectionClass object default property - used to say "__CLASS__"
--FILE--
<?php

class C {
public stdClass $a = FOO;
}
$reflector = new ReflectionClass(C::class);

define('FOO', new stdClass);
new C;

var_dump( (string)$reflector );

?>
--EXPECTF--
string(%d) "Class [ <user> class C ] {
@@ %sReflectionClass-class.php %d-%d

- Constants [0] {
}

- Static properties [0] {
}

- Static methods [0] {
}

- Properties [1] {
Property [ public stdClass $a = object(stdClass) ]
}

- Methods [0] {
}
}
"
19 changes: 19 additions & 0 deletions ext/reflection/tests/gh15902/ReflectionProperty-callable.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
--TEST--
ReflectionProperty object default - used to say "callable"
--FILE--
<?php

class C {
public stdClass $a = FOO;
}
define('FOO', new stdClass);

new C;

$reflector = new ReflectionProperty(C::class, 'a');
var_dump( (string)$reflector );

?>
--EXPECTF--
string(%d) "Property [ public stdClass $a = object(stdClass) ]
"
19 changes: 19 additions & 0 deletions ext/reflection/tests/gh15902/ReflectionProperty-class.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
--TEST--
ReflectionProperty object default - used to say "__CLASS__"
--FILE--
<?php

class C {
public stdClass $a = FOO;
}
$reflector = new ReflectionProperty(C::class, 'a');

define('FOO', new stdClass);
new C;

var_dump( (string)$reflector );

?>
--EXPECTF--
string(%d) "Property [ public stdClass $a = object(stdClass) ]
"
Loading