fix: enforce readonly declaration and inheritance rules - #66
Open
AlessioGiacobbe wants to merge 2 commits into
Open
fix: enforce readonly declaration and inheritance rules#66AlessioGiacobbe wants to merge 2 commits into
AlessioGiacobbe wants to merge 2 commits into
Conversation
The readonly checks previously lived only in the Native-class branch;
ZendVM-backed classes accepted declarations Zend rejects at compile
time. addClassProperty now enforces, for declared and promoted
properties alike (probed against Zend 8.4.13):
- readonly property with a default value ("Readonly property A::$x
cannot have default value") - a readonly property carries runtime
initialization state, so a compile-time default is meaningless
- untyped readonly property, including untyped promoted readonly ctor
params ("Readonly property A::$x must have type")
- static readonly ("Static property A::$x cannot be readonly")
- a `readonly class` applies the same three rules to every property:
the class-level Modifiers::READONLY flag (already recorded on
ClassDef->flags for the Translator-side inheritance check) is OR-ed
into the per-property check
Promoted readonly params keep accepting parameter defaults: the default
belongs to the constructor argument, not the property (Zend-verified).
The inheritance_error_prop_readonly fixture used `readonly int $x = 2`,
which Zend itself rejects with the default-value error before ever
reaching the readonly-mismatch link error; the default is dropped so the
fixture still exercises the inheritance mismatch.
Zend seals readonly-ness across a hierarchy: a non-readonly class cannot extend a readonly one and vice versa. Both directions compiled silently.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Readonly declaration rules were unchecked for ZendVM-backed classes (checks existed only in the Native-class branch, which rejects readonly wholesale):
public readonly int $x = 5;(Zend: "cannot have default value"), untyped readonly ("must have type"), and static readonly all compiled, for declared and promoted properties and viareadonly class— each rule probed for exact Zend behavior; promoted readonly params keep their legal parameter defaults.Readonly-class inheritance was also not sealed: a non-readonly class extending a readonly one (and the reverse) compiled; both directions now fail with Zend's messages.
One pre-existing fixture (inheritance_error_prop_readonly.php) used
readonly int $x = 2, which Zend itself rejects before the inheritance error under test; the default was dropped so the fixture still exercises the mismatch.Part of the split of #39.