Skip to content

Introduce ReflectionProperty::isUnset() #18620

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

Open
mvorisek opened this issue May 21, 2025 · 4 comments
Open

Introduce ReflectionProperty::isUnset() #18620

mvorisek opened this issue May 21, 2025 · 4 comments

Comments

@mvorisek
Copy link
Contributor

Description

There is a difference between uninitialized and unset property when magic method like __get() is defined. The need of these two states was discussed in #9389.

Currently there is no way to detect if property is just uninitialized or also unset, so from the observable state, it is impossible to tell the property behaviour of a given object.

repro: https://3v4l.org/lDeXi

This is a feature request to add ReflectionProperty::isUnset() method.

An alternative would be to update property_exists() function behaviour - https://www.php.net/manual/en/function.property-exists.php#116824 - which does not account for unset property since it was introduced.

@IMSoP
Copy link
Contributor

IMSoP commented May 26, 2025

PHP currently has a slightly bewildering range of states for properties, which all behave differently:

  • declared and assigned
  • declared without type and not yet assigned (possibly not a distinct state, just implicitly assigned null?)
  • undeclared
  • undeclared but created dynamically by assignment
  • declared with type and not yet assigned ("uninitialized")
  • declared without type but then unset (mostly invisible, but private or protected modifier is remembered)
  • declared with type but then unset (same as "uninitialized", but triggers __get hook)

Trying to give them all meaningful names or combinations of flags in reflection is tricky at best.

@mvorisek
Copy link
Contributor Author

Yes and as you used the term "unset" in your enumeration, isUnset should be a good name. isSet would be confusing, as unassigned property does not imply unset.

@IMSoP
Copy link
Contributor

IMSoP commented May 26, 2025

I used the word unset to mean the operation which you can perform on properties, not any one of the seven states.

Internally, the zval is set to a special type called IS_UNDEF, which is apparently what's already checked by ReflectionProperty::isInitialized https://heap.space/xref/php-src/ext/reflection/php_reflection.c?r=3f03f7ed3d988567b5a59ae542579fd91cdfde42#6292

if (EXPECTED(IS_VALID_PROPERTY_OFFSET(prop_offset))) {
	zval *value = OBJ_PROP(Z_OBJ_P(object), prop_offset);
	RETURN_BOOL(!Z_ISUNDEF_P(value));
}

It looks like this is the extra magic that lets typed properties do something different: https://heap.space/xref/php-src/Zend/zend_object_handlers.c?r=90d2f8abfd597fd9c1a6773f129831b33a8253c8#770

if (UNEXPECTED(Z_PROP_FLAG_P(retval) & IS_PROP_UNINIT)) {
	/* Skip __get() for uninitialized typed properties */
	goto uninit_error;
}

and in https://heap.space/xref/php-src/Zend/zend_types.h?r=6c09c167efe557e11f266f70a94767e6bcf92df8&fi=IS_PROP_UNINIT#IS_PROP_UNINIT

 /* Properties store a flag distinguishing unset and uninitialized properties
 * (both use IS_UNDEF type) in the Z_EXTRA space. As such we also need to copy
 * the Z_EXTRA space when copying property default values etc. We define separate
 * macros for this purpose, so this workaround is easier to remove in the future. */
#define IS_PROP_UNINIT (1<<0)
#define IS_PROP_REINITABLE (1<<1)  /* It has impact only on readonly properties */
#define IS_PROP_LAZY (1<<2)
#define Z_PROP_FLAG_P(z) Z_EXTRA_P(z)

If I'm reading that all correctly, the current "isInitialised" check will include any of "untyped property after unset()", "typed property before assignment", and "typed property after unset()".

So, if ReflectionProperty::isUnset meant "type is IS_UNDEF and flags do not contain IS_PROP_UNINIT, you'd have these combinations you could check for:

  • Declared without type, unset() has been called - ! $prop->hasType() && $prop->isUnset($obj) or equivalently ! $prop->hasType() && ! $prop->isInitialised($obj)
  • Declared with type, never been assigned - $prop->hasType() && ! $prop->isInitialised($obj) && ! $prop->isUnset($obj)
  • Declared with type, unset() has been called - $prop->hasType() && $prop->isUnset($obj)

That technically covers the bases, but it's not particularly easy to follow. I'm not sure if there are extra cases needed to account for hooked properties, readonly, etc

@mvorisek
Copy link
Contributor Author

100% agree and perfect analysis! ❤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants