[ndossche]

For private fields, the scope has to be taken into account, otherwise the property info may come from the wrong ce.
trace_ce is the class entry for b in the test, so we look up the property info from the wrong ce (should be a).

Maybe this can be improved further by checking is_derived_class(ce, scope) and then reading the property info from the scope, e.g. by using return zend_get_parent_private_property(op_array->scope, ce, member); instead of return NULL? But that API isn't public though.

[ndossche]

Maybe this can be improved further by checking is_derived_class(ce, scope) and then reading the property info from the scope, e.g. by using return zend_get_parent_private_property(op_array->scope, ce, member); instead of return NULL? But that API isn't public though.

Here's how that idea would look like (apply patch on top of this PR):

https://gist.github.com/nielsdos/3090a86982cba83124ffc8715ff3e031

[dstogov]

Great work!

[ndossche]

Thank you for reviewing.
@dstogov my PR returns NULL in the else branch. I proposed https://gist.github.com/nielsdos/3090a86982cba83124ffc8715ff3e031 additionally. But that changes the API (makes it public) so that is probably not ok for stable branches. Do you think the additional patch may be applied to master?

[dstogov]

Thank you for reviewing. @dstogov my PR returns NULL in the else branch. I proposed https://gist.github.com/nielsdos/3090a86982cba83124ffc8715ff3e031 additionally. But that changes the API (makes it public) so that is probably not ok for stable branches. Do you think the additional patch may be applied to master?

Yes. It's not OK to break API in old versions.
I didn't understand why do you use zend_get_parent_private_property(). Can you explain?
May be it's better to just commit this and then create new PE for master with explanation test.

[ndossche]

Merged as-is.

I didn't understand why do you use zend_get_parent_private_property(). Can you explain?

Please disregard, I see now this is wrong, I guess I was confused.
Thanks for checking.