Repository navigation
fix(compiler): keep typed property is-a check for unknown object values - #156
Open
alwaysLinger wants to merge 1 commit into
Open
alwaysLinger wants to merge 1 commit into
alwaysLinger wants to merge 1 commit into
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
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.
Observed failure
Assigning a value that is statically typed only as
objectto a property declared with a specific class compiles without a diagnostic and runs without a runtime check: a wrong-class object is stored in the typed property slot. ZendPHP rejects the same assignment with aTypeErrorat runtime.Because AOT object properties are fixed-layout C++ slots, the stored object is later read as the declared class, so every subsequent typed access operates on the wrong object layout.
ZendPHP 8.4:
TypePHP master (65d2ea7):
Reproduction
Save the file above as
repro.phpand compare:Right values typed
mixedor produced bystd::any()already generate the runtime is-a check and fail with the expectedTypeError;objectis the only static type that silently bypasses it.Root cause
assertCanAssignObjectProperty()defers towrapObjectPropertyAssignTypeCheck()when the right side's class is statically unknown (theTODO: ... a runtime check is requiredbranch). The deferred check is then skipped:wrapObjectPropertyAssignTypeCheck()returns early whencanAssignStaticTypeToObjectProperty($def, $rightType)is true;canAssignStaticTypeToObjectProperty()compares storage types only (default => $rightType === $def->type) and never consults$def->class;Foois stored asType::OBJECT, and anobjectparameter is detected asType::OBJECT, soType::OBJECT === Type::OBJECTholds and the runtime check is dropped.The generated C++ for the repro is a bare attribute write with no check:
while the same assignment with a
mixedright value correctly generates:Fix
src/Parser/PropertyAccessTrait.php(+9/-2): inwrapObjectPropertyAssignTypeCheck(), do not take the static-assignable early return when the right side isType::OBJECTwith a statically unknown class and the property declares a specific class. Those writes fall through to the existing runtime is-a check generation used forType::VARright values.canAssignStaticTypeToObjectProperty()itself is unchanged: its other two callers (assertCanAssignObjectProperty()and the compound-assignment path inAssignOpTrait) rely on the storage-type verdict to raise compile-time fatals, and changing it globally would reject valid programs whoseobjectvalue happens to match at runtime. Assignments that are statically provable (subclass to base, known compatible classes) still compile with no runtime check.