Skip to content

Fix setRawValueWithoutLazyInitialization() / skipLazyInitialization() on initialized proxy#16344

Closed
arnaud-lb wants to merge 1 commit intophp:PHP-8.4from
arnaud-lb:fix-skip-init
Closed

Fix setRawValueWithoutLazyInitialization() / skipLazyInitialization() on initialized proxy#16344
arnaud-lb wants to merge 1 commit intophp:PHP-8.4from
arnaud-lb:fix-skip-init

Conversation

@arnaud-lb
Copy link
Copy Markdown
Member

skipLazyInitialization() removes the lazy flag of a property and initializes it to its default value. If the property was already initialized, it has no effect.

However, initialized proxies have all their properties marked as lazy, but skipLazyInitialization() should not initialize them.

Similarly, setRawValueWithoutLazyInitialization() should not set the value on initialized proxies.

Here I make sure to produce the effect on the real instance rather than the proxy.

NB: It's possible that the real instance is also a proxy. In this case I fetch its real instance again, and the affected object is the deepest instance.

@arnaud-lb arnaud-lb marked this pull request as ready for review October 10, 2024 16:26
@arnaud-lb arnaud-lb requested a review from iluuu1994 November 4, 2024 17:08
Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

Looks good otherwise. Sorry for the late response. I guess it's too late for 8.4 but this seems like a fringe case.

bool prop_was_lazy = (Z_PROP_FLAG_P(dst) & IS_PROP_LAZY);

if (!(Z_PROP_FLAG_P(dst) & IS_PROP_LAZY)) {
if (!prop_was_lazy) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

prop_was_lazy is used below, and always true. Is this correct? In that case, it should be dropped from the condition.

@arnaud-lb
Copy link
Copy Markdown
Member Author

@iluuu1994 just to be sure, you meant 8.4.0, not 8.4?

@iluuu1994
Copy link
Copy Markdown
Member

@arnaud-lb Yes. Sorry for the imprecise wording.

@arnaud-lb
Copy link
Copy Markdown
Member Author

No worries! Thank you for the review

@arnaud-lb arnaud-lb closed this in c310be0 Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants