Don't call __set() on uninitialized typed properties #4854
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is a followup to #4696. cc @marandall @trowski @morrisonlevi |
Ocramius
approved these changes
Oct 24, 2019
| <?php | ||
|
|
||
| class Test { | ||
| public int $foo; |
There was a problem hiding this comment.
Could you add a field that is not typed, so we verify the behavior for that as well? It shouldn't be an issue, but it does indeed prevent a regression
trowski
approved these changes
Oct 24, 2019
|
Closing in favor of #4856. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New stab at bug #78226: We introduce a type flag that distinguishes an uninitialized typed property and a property that has been explicitly
unset(). Assigning to an uninitialized property will initialize the property and not invoke__set(). Assigning to an explicitly unset property will invoke__set().This should be a compromise that both avoids WTF when using typed properties in a class with __set(), while still keeping @Ocramius's ProxyManager code working.
The patch turned out to be fairly non-intrusive, but it does disable an optimization for refcounted checks.