merge: cascade 8.4 into master (waves 2-3, conflicts resolved) - #219
Merged
Conversation
… helper getDefaultProperties() and getDefaultStaticMembers() were the same method twice: each built a closure-generator over its zval table only to iterator_to_array() it straight back. Both now delegate to a single private readZvalTable() that wraps the contiguous table in a bounds-checked Type\StructArray and returns the ReflectionValue array directly. Public signatures are untouched - getDefaultProperties() keeps its `: iterable` declaration (public API) and both keep returning an index-keyed array. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
installExtensionHandlers() was ~120 lines of the same block repeated 23 times
(`if ($this->implementsInterface(X::class)) { $h = parent::getMethod('__y')
->getClosure(); $this->setZHandler($h); }`). The interface/magic-method/installer
triples now live in one EXTENSION_HANDLERS const and a single loop walks it, so
registration order is the map's insertion order (ObjectCreateInterface first, as
before) and supporting a new hook interface is one map line.
The twenty-one setXxxHandler() methods had byte-identical bodies as well. Every
public method and its precise hook return type is kept - that is the API - but the
body is now one call to a private installObjectHook(class-string<THook>, Closure):
THook, so PHPStan still resolves each setter to its concrete hook class at level max.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Twenty-two hooks hand-rolled the same prologue before invoking the captured engine pointer: a hasOriginalHandler() check throwing a literal LogicException, followed by an untyped `($this->originalHandler)(...)` invocation that static analysis can only see as "trying to invoke FFI\CData". AbstractHook::getOriginalCallable() already is that prologue - it throws the exact same exception and hands back a value narrowed to `callable` - and seven hooks were converted to it already (five of them kept the now-dead guard in front of the call). The remaining ones are converted here and the dead guards are dropped, so "no original handler" is worded in exactly one place. CreateObjectHook keeps its two-branch shape (it falls back to ReflectionClass::newInstanceRaw when no handler was captured) but asks hasOriginalHandler() instead of comparing the raw property, and takes the handler through the same accessor. Twelve callable.nonCallable baseline entries and two inline @PHPStan-Ignore comments become unnecessary and are removed. No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…rray Three one-line helpers wrote the same pointer-slot store by hand: ReflectionClass::storeAdaptationListSlot(), ReflectionClass::storePropertyInfoSlot() and ClassSpecializer::storePointerSlot(). None of them bound-checked the slot even where the block size was known at the call site. They are replaced by one owner on Type\StructArray, next to the existing replace(): storePointer() overwrites the pointer a slot carries (replace() byte-copies INTO the struct a slot points at, which is a different operation), and both now share one assertInBounds() so every write is checked on both ends. Every call site builds the view over the block it just allocated, with the size it allocated it with. precedenceExcludeNames() existed verbatim in both ReflectionClass and ClassSpecializer. ReflectionClass owns zend_class_entry and its trait adaptations and ClassSpecializer already depends on it, so the accessor stays there as an @internal static and the specializer calls it. It now returns a StructArray sized by the entry's own num_excludes instead of a freely indexable raw pointer, so the four call sites iterate it instead of re-deriving the loop bound. The nullable properties_info_table READS stay raw on purpose: a nullable pointer array is a shape the element-typed view does not model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…hecked view Two hand-rolled struct-array walks indexed raw pointers while the element count was right there: - ClassSpecializer::everyReturnIsVerified() read $opArray->opcodes[$index] and [$index - 1] directly although $opArray->last is the opcode count. It now iterates a StructArray built from that count, the same shape FunctionLikeTrait::getOpCodes() already uses; the pre-existing `$index === 0` guard still short-circuits before the predecessor lookup. - ReflectionProperty::getHook() indexed the zend_property_info hooks block by hook kind with no bound at all. It now goes through a StructArray sized by Core::ZEND_PROPERTY_HOOK_COUNT. Behaviour is unchanged for every in-range index; an out-of-range one now raises OutOfBoundsException instead of reading engine memory past the block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…guarantee Half the hook family declared `public function proceed()` with no return type at all, so every consumer of a proceed() result had to guess and PHPStan carried a missingType.return entry per method. The types are derivable: each class documents the C callback typedef it implements, and the value proceed() hands back either comes straight out of that engine handler (guaranteed shape: FFI returns an int for an int-returning function pointer and CData-or-null for a pointer-returning one) or out of a ReflectionValue accessor. Declared accordingly - int for the verdict-returning handlers (compare, do_operation, interface_gets_implemented, cast_object), ?object for the pointer-returning ones (get_property_ptr_ptr, get_properties_for), object for write_property (the standard handler reports the written slot or &EG(error_zval), never NULL), void for unset_property and zend_ast_process, mixed for the accessors that materialize an engine value into a PHP one. One deliberate exception: GetPropertyPointerHook::handle() stays `mixed`, because its value comes from the USER handler rather than the engine. A stricter declaration would turn a userland contract violation into a TypeError raised inside an FFI trampoline, where nothing can catch it (issue #50). 21 more baseline entries (missingType.return plus two return.type) drop out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…tions getChildren()/getChild()/replaceChild()/removeChild() each repeated the same four-line bounds-and-cast block, and NONE of them checked for a NEGATIVE index: `$index >= $totalChildren` lets -1 through, so the three writers read and overwrote engine memory in FRONT of the node's own zend_ast. All four now go through one private child-slot accessor built on Type\StructArray, which checks both ends of the range; the out-of-range message keeps its previous wording (with the offending index added). Node::replaceChild() and ListNode::append() also reached `$node->node` - a PROTECTED property - on a parameter typed as the NodeInterface INTERFACE. Any NodeInterface implementation outside the Node hierarchy made that a fatal error. Narrowing the parameter type would be a BC break, so the reach goes through a shared Node::rawNodeOf() guard that throws a descriptive \InvalidArgumentException instead. The public signatures are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…ublic constants Five classes rendered an engine number as the constant declaring it, each with its own cache idiom (a static property, a `static $x` local, a hand-built loop) and each over an UNFILTERED getConstants(). Type\ConstantNames now owns the mapping: public constants only (ReflectionClassConstant::IS_PUBLIC), integer values only, cached per class and filter, with the miss behaviour left where it belongs - ReflectionValue::name(), NodeKind::name() and OpCode::name() still throw, OpLine::typeName() and LiveRange::kindName() still fall back to 'UNKNOWN'. The filter fixes a latent bug in NodeKind::name(): the private AST_SPECIAL_SHIFT / AST_IS_LIST_SHIFT / AST_NUM_CHILDREN_SHIFT constants (6, 7, 8) were flipped into the map alongside the real node kinds, so NodeKind::name(6) answered 'AST_SPECIAL_SHIFT' instead of reporting an unknown kind. OpCode::name() got the same filter. LiveRange keeps excluding KIND_MASK (a public mask, not a kind) explicitly. Values declared twice still resolve to the last declaration, exactly like the array_flip() calls this replaces - the zval type ids reuse their numbers on purpose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…ances
setPublic()/setProtected()/setPrivate() were copy-pasted across ReflectionMethod,
ReflectionProperty and ReflectionClassConstant - nine methods doing the same
`&= ~ZEND_ACC_PPP_MASK; |= X` in three different engine fields, and ReflectionMethod's
three resolved getCommonPointer() twice per call. They now come from one
AccessFlagsTrait; each class only declares WHERE its flags live by implementing
replaceAccessFlags(), which does the read/modify/write in a single pass.
`if ($on) { $f |= M; } else { $f &= ~M; }` appeared seven more times
(ReflectionMethod::setFinal/setAbstract/setStatic, ReflectionProperty::setStatic,
FunctionLikeTrait::setDeprecated/setVariadic/setGenerator/setClosureFlag). Those are now
one-liners over setAccessFlag()/setFunctionFlag(), the latter private to FunctionLikeTrait
so plain ReflectionFunction gets it too.
Public signatures are unchanged - verified that every setter is still a public method of
the same class after the trait composition.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Swapping the module_registry key for a persistent interned block made AbstractModule walk nNumUsed/arData/bucket->key by hand - engine-struct surgery performed outside the class that owns the structure, which AGENTS.md keeps inside the owning type. HashTable now holds the framework's only bucket walk and exposes it as a pair of methods: findKeyEntry() returns the engine's OWN key block (so a caller can inspect its storage class - interned, permanent, persistent) and replaceKey() exchanges that block for an equivalent one, rejecting a replacement whose content would change the bucket hash. count() joins them so the live element count of a table is read through Countable instead of reaching for nNumOfElements. makeRegistryKeyPersistent() is rewritten on the pair and keeps its exact semantics: a permanent key (registered during engine startup) is left alone and the persistent interned replacement is only minted when a swap is actually needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
… owning classes The persistent-heap machinery reads and writes engine structures the Type/Reflection layer owns. These are the typed accessors it needs, so the call sites can stop poking fields (converted in the following commit): - ObjectEntry::getPropertySlotCount() - the inline properties_table slot count of the object's CURRENT class entry, the loop bound that belongs next to getPropertySlot() (which now reads it instead of dereferencing ce a second time); - ReflectionClass::getDefaultPropertiesCount() - the same number taken from a resolved zend_class_entry, for machinery whose object still carries a stale ce; companion of the existing getObjectSize() static; - ReflectionValue::setUncountedPayload() - a payload pointer plus the complete u1.type_info word, written WITHOUT any refcounting: the shape persistent interned strings, sealed arrays and refcount-pinned objects need. PersistentObjectFactory::persistentClone() accepts the stub-typed zend_object view its callers now hold, which frees two baseline entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…d struct stubs
The hook layer is the one subsystem that missed the struct-stub cutover: every
engine pointer a hook holds was declared as a bare FFI\CData, and since the
libffi trampoline delivers the callback arguments as `mixed`, each assignment
produced an "(FFI\CData) does not accept mixed" entry in the baseline - 84 of
them across src/ClassExtension/Hook, a fifth of the whole file.
Applied the convention from AGENTS.md ("Engine structs are typed by generated
stub classes"), exactly as the Reflection\* and Type\* layers already do it:
the field is declared `object` and carries a `/** @var <stub> */` docblock
naming its ZEngine\Generated view, and handle() - the single boundary where the
raw callback arguments arrive - narrows them once with a `@var` block over the
destructuring. That narrowing replaces the `assert($x instanceof CData)`
scaffolding the same statements used to carry (AGENTS.md calls out that weak
form as what the stub migration supersedes); the asserts that check real engine
or handler invariants stay.
Fields the stubs deliberately do not model keep their CData declaration with a
note saying why: the `void **` property cache slot, the `int *`/`zend_long *`
out-parameters of get_debug_info and count_elements, and the zend_object** /
zend_function** / zend_class_entry** double pointers of get_method and
get_closure (the stubs model structs, not pointer-to-pointer handles).
AstProcessHook keeps a raw zend_ast handle too, because the AST wrappers that
own that struct (NodeFactory/Node) have not been migrated yet.
Two owning boundaries were widened to accept a stub view alongside CData, the
usual `@param CData|Stub` shape: Executor::setFakeScope()/withFakeScope() (every
property hook now passes a typed `$object->ce`) and
ReflectionClass::newInstanceRaw().
Also removed CompareValuesHook::$returnValue, a protected property that was
declared but never read or written - compare_values has no result slot.
52 more baseline entries drop out; hook entries are down from 91 to 6.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
PersistentHeap and PersistentGraphCloner hand-rolled the pointer arithmetic and field
pokes that the Type/Reflection layer already owns - three copies of the same
properties_table dig, direct gc->refcount, ->properties, ->handle and ->ce writes, and
a second array iterator in Memory/. All of it now goes through the owning API:
- inline property slots: ObjectEntry::getPropertySlot()/getPropertySlotCount() replace
the `Core::cast('zval *', ...)` table digs, and ReflectionValue::getBaseType() plus
the typed getRawString()/getRawArray()/getRawObject() replace the u1->v->type and
zend_value member reads. The heap's re-attachment pass keeps taking the slot COUNT
from the class entry it resolved itself (a stored object's own ce is stale until
pass 3 rewrites it) and reaches the table through the existing
getPropertyTablePointer() + StructArray view;
- object fields: ObjectEntry::getReferenceCount() for the in-use guard,
setHandle()/getHandle(), get/setDynamicPropertiesPointer() and setClass() for the
re-attachment writes, getNativeValue() for the root alias;
- payload rewriting: ReflectionValue::setUncountedPayload() instead of writing
zend_value members and type_info by hand;
- array iteration: PersistentGraphCloner::walkArray() is gone. Its reason for existing
("HashTable::getIterator drops integer keys of hashed tables") stopped being true when
a56d59d taught the iterator to read integer keys from the bucket hash field - the two
landed on parallel branches, so the stale duplicate survived the merge. Both call
sites now foreach over a borrowed HashTable view;
- element counts: HashTable::count() instead of eight nNumOfElements reads.
With the digs gone both files' identical asCData()/asInt() narrowing helpers have no
callers left and are deleted rather than consolidated. Four baseline entries are freed
by the honest parameter types the conversion needs.
Behaviour is unchanged throughout: same field reads, same write order, same guards.
The added wrapper allocations are on the persist/attach paths (once per object or
array), never per opcode.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Nine ignore entries no longer match any reported error and two counts dropped, all as a direct consequence of the preceding commits: - the two `NodeInterface::$node` property.notFound ignores (Node, ListNode) are gone because the protected reach now goes through the instanceof-guarded accessor; - the three NodeFactory::fromCData() "mixed given" ignores in Node.php are gone because the child reads are narrowed once in childAt(); - Core::cast() "mixed given" drops from 5 to 1 in Node.php and 3 to 2 in ListNode.php, the casts having moved behind the struct-array accessor; - the four array_flip() ignores (NodeKind, ReflectionValue, OpCode, OpLine) and OpLine's "Cannot access offset int on mixed" / typeName() return.type ignores are gone with the shared, properly typed ConstantNames lookup. Nothing was added to the baseline. `composer phpstan` and `composer cs:check` are clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…tructors The heap already raises every typed failure through a factory on its exception class (HeapInertException::create(), GraphCorruptedException::forSlot(), ...); these two inline throws were the exceptions to the rule inside PersistentHeap. Their message text moves onto PersistentHeapException as heapDestroyed() and corruptMetadata(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
AbstractMethodResolutionHook::resolveRawFunction() reached into a class entry by hand: it looked the entry up in the engine class table, read ->function_table off the raw struct, took its address and built a Type\HashTable around it - the exact "callers never reach into a raw CData" pattern AGENTS.md forbids, and a duplicate of what ReflectionClass already does when it is constructed. It now goes through the owning object: `new ReflectionClass($className)` does the class-table lookup and raises the "should be in the engine" ReflectionException itself, and getMethodTable() is the accessor that owns ce->function_table. Eleven lines become three, and no call site outside ReflectionClass touches zend_class_entry.fields any more. Note on the alternative: PR #196 proposed a static entry-level helper (ReflectionClass::entryMethodTable()) for this shape. That helper is being withdrawn as against the framework's object model - consumers hold Reflection* objects, not raw entries plus static utilities - so this uses the instance API and does not depend on that PR. The extra cost (a native reflection constructor and three unused table wrappers) is paid only on the slow path: a wrapper produced by proceed() short-circuits on the identity fast path above, and the VM inline-caches the resolved function per call site for compile-time constant method names. While here, $proceedRawFunction and the two handle() return docs get their zend_function|zend_internal_function stub views, retiring the last hook return.type baseline entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
IteratorBridge kept the per-iteration state as an array shape
(`array{iterator: Iterator, pointer: CData, broken: bool}`). PHP arrays are
value types, so what each vtable callback read out of the registry was a
snapshot: marking an iteration broken could not be done on the record in hand
and had to go back through a second registry lookup by address, with an isset()
recheck around it - a shape that only existed to work around the copy.
The record is now a tiny `@internal` BridgedIterator with public typed
properties, so it is shared by reference. Every callback preamble is one lookup,
breakIteration() takes the state it is given and writes the flag straight
through it, and the iterator/pointer pair is readonly and named rather than
string-keyed.
Behavior is unchanged for every iteration path; the one difference is in the
pathological case where the engine iterator is destroyed from inside the
userland callback that then throws - the swallowed Throwable is now reported as
the E_USER_WARNING it always should have been, instead of being dropped because
the registry entry had already gone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Every concrete hook in ClassExtension\Hook and System\Hook is a leaf: it implements one specific engine callback typedef and is instantiated by the extension machinery, never derived from. Nothing in src/ or tests/ extends one, and the two hooks that already implement HookInterface directly (OpCodeHook, IteratorBridge) have been final since they were written. Marking the remaining 27 final makes the extension point explicit - hooks are customized by passing a user handler closure, not by subclassing - and keeps the Abstract* bases as the only place a new hook shape can be introduced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
src/AbstractSyntaxTree/ had no direct tests: the only existing reference was EngineConstantsTest cross-checking the NodeKind constant values against the generated ground truth. Nothing exercised the wrappers that decode and walk a real zend_ast. Two suites, split by what they touch: - NodeKindTest (default group) covers the pure bit arithmetic over the zend_ast_kind encoding - isSpecial()/isList()/childrenCount() for a spread of kinds from 0 to 4 children, the invariant that no kind is both a list and a special node (NodeFactory checks the special bit first, so an overlap would cast a list into a zend_ast_decl), plus name() resolution, its lazy cache and its rejection of an unknown kind. - NodeTest (internal group, process-isolated) drives Core::$compiler->parseString() and covers the wrappers over the resulting tree: the AST_STMT_LIST root, the empty list, children order and per-node line numbers, getChild()/getChildren() agreement, the OutOfBoundsException raised by getChild()/replaceChild()/removeChild() at or beyond the children count, NodeFactory dispatch into ValueNode / DeclarationNode / ListNode / plain Node, the zval extra-slot line of a value node, the four fixed child slots of a declaration, setLine()/setAttributes() write-through, the replaceChild() swap and the removeChild() detach, and dump() with and without indentation. The parse-driven suite is classified like every other test that mutates the compiler globals and manages engine memory (System\CompilerTest, Memory\MemoryLeakScenarioTest): parseString() writes CG(ast)/CG(ast_arena) and the lexical state, and releases the tree through AstOwnership - zend_ast_destroy plus a Core::free() of the arena buffer. AstOwnership also dictates what the tests may do with the tree: the wrappers are never constructed directly (the constructors call zend_ast_create_*, which allocates from CG(ast_arena) and is only valid mid-parse), a node is never left detached (its payload references would never be released) and never linked in twice (it would be destroyed twice). removeChild() is therefore always paired with a re-attach, and replaceChild() only permutes existing children. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
The `performance` group is excluded from phpunit.xml.dist and had no composer script and no CI invocation, so it never ran anywhere. Its only test, tests/Performance/GeneratedFunctionBenchmarkTest.php, is the only thing that defends the README's "zero FFI at call time" claim: it asserts that a function published with ReflectionFunction::addFunction() dispatches through the normal Zend VM (Path B in docs/memory-model.md) rather than through an FFI trampoline. - composer test:performance runs the group with --fail-on-skipped, mirroring the opcache scripts, so a future self-skip cannot read as a pass. - .github/workflows/performance.yml is a new file - ci.yml is left untouched because PR #198 owns it. It mirrors ci.yml's trigger shape (pull_request plus push on the version branches), its PHP_MINOR env and its setup-php usage with plain version tags, and carries the repository standard being established in #198: permissions: contents: read and a per-ref concurrency group that cancels superseded pull-request runs only. The job is deliberately INFORMATIONAL (continue-on-error at job level plus a summary step) rather than a merge gate: the benchmark's verdict is a wall-clock ratio between a generated and a hand-declared function, which is sound on a quiet machine but noisy on a shared GitHub runner. A red run here means "read the [generated-function] numbers", not "this pull request is broken". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
The suite only failed on warnings; deprecations, notices, risky tests and stray output were displayed and then scrolled past in the CI log. For a library that writes into engine memory those are signals about the runtime, not noise - a deprecation raised from a hooked engine path or a test that unexpectedly prints is exactly the kind of thing this suite exists to notice. Adds failOnDeprecation, failOnNotice, failOnRisky and beStrictAboutOutputDuringTests to phpunit.xml.dist. This may turn CI legs red that were quietly green. That is the point: if a leg fails on one of these, the failing signal gets triaged - fixed, or explicitly documented as expected - and the flag stays. .gitignore is left alone: PHPUnit 12 still writes its result cache to `.phpunit.result.cache` when no cacheDirectory is configured (verified against Runner\ResultCache\DefaultResultCache in the installed 12.x), so that entry is not stale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
beStrictAboutOutputDuringTests (added in this PR) correctly flagged the test as risky on every CI leg: the var_dump(42) call that triggers the E_DEPRECATED also prints to stdout. Buffering the dump keeps the deprecation probe while swallowing the output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Every test method the baseline flagged with a missingType.* error now
declares its parameter, return and property types: 49 baselined entries
across tests/System/ExecutionDataTest.php, tests/Reflection/*Test.php,
tests/Type/ResourceEntryTest.php and tests/Stub/NativeNumber.php.
Data providers carry precise iterable phpdoc (list<array{mixed, int}> and
friends) instead of a bare `array`, which also lets the analyser check the
provider rows against the test signatures.
Typing NativeNumber's numeric payload as int|float|numeric-string and the
ResourceEntryTest handle as a real resource resolved eight further baselined
entries (the four binaryOp.invalid arithmetic errors, the getNumericValue
return type, a @PARAM tag naming a parameter that never existed, and the
fclose()/string-cast argument types); they are pruned as well.
Baseline: 401 entries / 475 errors -> 344 / 418. Types only, no behavioural
change: the single added statement is a setUp() guard asserting the fixture
stream actually opened, which the new `resource` property type requires.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Compiler::parseString() looked like it could never return a tree: CG(ast) is cleared right before the parse, so the analyser folded every later `$ast === null` check to true and declared the return statement unreachable. The field read now goes through a private getRawAST() accessor that guards the CData invariant once, the way getActiveOpArray() already does - getAST() and the catch path use it too, which also drops a baselined argument.type. Behaviour is unchanged; the parse result is still whatever zendparse() left in CG(ast). ExecutionData::getCallVariableByNumber() carried a @PARAM for a $call parameter it has not had since the method stopped being static. Core::cast()'s count() probe keeps its deliberate discard - the answer it wants is the FFI\Exception, not the number - and now says so through a scoped @PHPStan-Ignore instead of a baseline line. The @var on the preload iterator moves onto the value it actually describes (SplFileInfo), since the iterator itself was never an array. The five "unused" constructor parameters (ValueNode, ReflectionValue, ClosureEntry, ReferenceEntry, StringEntry) are all load-bearing: each constructor reads its own frame's argument slot 0 to capture the caller's zval, so the parameter is the mechanism and dropping it would be both a BC break and a functional one. They keep their signatures and now carry the explanation plus a scoped ignore at the site. Baseline: 344 entries / 418 errors -> 333 / 406. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
bootstrap.php is executed in every consumer process (composer autoload.files) and preload.php is the documented opcache.preload entry point, yet neither was covered by PHPStan or php-cs-fixer - the proof was preload.php's `__DIR__.'/vendor/autoload.php'`, unspaced concatenation that @PER-CS2.0 forbids everywhere else in the repository. Both are added to the analysed paths, together with tools/generator/symbols.php (the generator manifest) and tools/examples/worker-loop.php (the soak-test gate). The fixer finder gains the two root scripts and this config file itself via ->append(). Fallout was small: an alignment fix in bootstrap.php, the concatenation and a missing blank line in preload.php. worker-loop.php contributes seven baseline entries, all raw CData dynamics of exactly the kind already baselined for the equivalent tests/Memory/scenarios files (hook proceed() results, persistent clone handles, array-decay indexes). Baseline: 333 entries / 406 errors -> 340 / 414. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
The $header the fixer config builds was never used by any rule. Wiring it to header_comment turns out to be blocked on a question this PR cannot answer for the maintainer: the fixer enforces one literal header, and the corpus carries three copyright years - 2019 (89 files), 2020 (29) and 2026 (168). Whichever year the rule hardcodes rewrites the @copyright line of every file carrying another: 118 files for 2026, 210 for 2019, 270 for 2020. Every one of those sets includes files owned by open pull requests (#199, #201, #203, #204, #205). What was in the way besides the year is now fixed. Eleven files the fixer lints carried no license header at all, and nine of them opened with a descriptive file docblock that header_comment would have silently replaced - the generator driver's usage text, the emitters, the manifest. They now carry the header above their own docblock, so the rule would be additive rather than destructive. Two files (ObjectStoreTest.php, preload.php) diverged in shape by one line and are normalized. The corpus is therefore uniform except for the year, and the config documents the exact four-line rule to enable once that is settled. The two OpCache payload fixtures are deliberately left alone: their byte content is what the file-cache tests compile and compare. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
phpstan/phpstan-phpunit joins require-dev and its extension.neon/rules.neon are included explicitly, matching how every other service in the config is declared (no extension-installer). The type extension makes assertions narrow, and the rule set reads the suite for assertion smells. Two of those smells were real and are fixed: an assertSame(true, ...) that wanted assertTrue(), and a redundant assertInstanceOf() on an array element the return type already types (which also freed an import). Two more assertions that only held because the analyser folded a local assignment now read the value back through reflection - which is what the specializer tests meant to check anyway, since the slot is the thing being rewritten. The assertCount suggestion is declined for ReflectionClassDimensionTest: those tests exist to prove a bare count($object) reaches the engine's count_elements handler, and assertCount() would measure PHPUnit's constraint instead of the language construct under test. That is a config-level ignore with the reason attached. The remaining 31 findings are baselined. They split in two: assertions pinning a truth the engine creates at runtime and static analysis cannot see (an interface added to a compiled class, a constant made private, $this replaced in the live frame, a runtime-generated specialized class), and type guards the suite keeps as documentation. Three sites where the cause is local and specific carry an inline @PHPStan-Ignore with the reason instead. Baseline: 340 entries / 414 errors -> 365 / 444 (one entry, the assertIsObject narrowing, is resolved by the extension and pruned). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Nineteen sites carried two consecutive docblocks with nothing between them. PHP attaches only the second, so the first - always the prose explaining what the member is for - was invisible to every IDE and documentation tool while looking perfectly fine in the editor. Each pair is now one block: the prose, a blank line, then the tags. Merged: src/Reflection/ReflectionValue.php (12 - getRawClass, getRawFunction, getRawString, getRawArray, getRawObject, getRawResource, getRawReference, getRawValue, setPointer, zvalPointer, buildTypeInfo, copyAndReleasePrevious), src/Core.php (3 - isTrackedBlock, untrackAndFree, untrack) and src/Type/ResourceEntry.php (fromCData). The three generator entry points hit the same shape from the other direction: the license header added in this branch sat directly above their descriptive file docblock, shadowing it. Their description moves below declare(), which is the shape symbols.php and the emitter libraries already use. Twelve further sites are left alone because their files belong to open pull requests: PayloadRelocator (2), ObjectEntry (2), FunctionLikeTrait (1), ReflectionMethod (2), ClassSpecializer (2) and ReflectionClass (3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…stants Per maintainer review on #203: constants stay at the top of the class, not interleaved with methods. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Makes the CI signal concrete on the exact combination the merge preview tests: this branch's dedup plus the merged wave (frame-scope resolution, sizeOfType migration, exception factories). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…plementations" This reverts commit cd0ab27.
…bounds-checked view" This reverts commit 7e30c26.
… StructArray" This reverts commit 4d9f958.
The StructArray adoption is reverted per maintainer review (too much internal churn for this PR); the real defect it fixed stays: the three child-slot writers accepted a negative index and reached engine memory below the table. The restored AST baseline stanzas match origin/8.4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Per review on #213: no cast in Compiler::getAST() and no asserts around the FFI boundaries - NodeFactory::fromCData()/Node::fromCData() declare the CData|zend_ast union with a single stub narrowing inside, and the Core boot plus the arena factory narrow with @var docblocks instead of runtime asserts. Baseline follows the message drift of the widened signatures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
phpstan-nobaseline.neon was an unreferenced audit convenience duplicating the dist config without the baseline include; the same audit is one temporary edit of phpstan.dist.neon away when needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…-6pfu7y-reflection-dedup refactor(reflection): deduplicate handler installation and flag setters, fix unchecked AST child index
Both sides of each conflict are fixes: 8.4's ConstantNames removed the array_flip errors, this branch's constructor documentation removed the unusedParameter ones - the merged baseline drops both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Executor's class/function/constant tables and object store, and Compiler's class/function tables, are bound to their engine-globals table once in the constructor and are only ever read afterwards - yet any consumer could overwrite them and silently detach the whole process from the engine table the wrapper is supposed to reflect. PHP 8.4 asymmetric visibility expresses that exactly: `public private(set)` keeps every existing read (and every mutation performed *through* the view) working unchanged while the binding itself becomes unswappable. No call site in src/ or tests/ ever assigned to these properties, so this is a pure tightening with no behavioural change. Core::$executor/$compiler/$modules stay untouched: they are static properties, for which asymmetric visibility does not exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Every method in src/ that was verified (by reflection against the actual parent/interface declaration) to override a parent method or implement an interface method now carries #[\Override]. This project pins one PHP minor and overrides native reflection methods whose signatures shift between minors - the attribute turns that silent drift into a compile error. Constructors and trait-provided implementations are deliberately left out: LSP does not apply to constructors, and a trait cannot know whether the using class has a parent to override. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
The ABI-mirrored engine values and the internal slot/flag constants stay constants (nothing is converted to an enum here) - they just carry their type now, so a wrong-typed override or a bad regenerated value is a compile error instead of a silent int/string mix-up. AbstractHook::HOOK_FIELD loses its now-redundant @var docblock. Array-valued constants are left untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Three closed sets that z-engine owns (as opposed to the ABI values mirrored from the engine headers, which stay class constants) become backed enums, in the style already set by ClassExtension\Hook\CastType and PropertyPurpose: - Type\SendMode for the three argument send modes decoded from the two bits above _ZEND_SEND_MODE_SHIFT. ArgumentEntry::sendMode() returns it and the strict from() now also guards the decode: the fourth bit pattern the field can physically hold is one the engine never writes, so it means the entry was read at a wrong offset. - EngineExtension\DependencyType and VersionRelation for the module dependency declaration. Both enums own the normalization of the legacy scalar input (fromValue()), which replaces the hand-rolled in_array() validation blocks in the ModuleDependency constructor while keeping the exact same InvalidArgumentException contract for an unknown type or relation. - Memory\DescriptorSlot for the persistent-heap descriptor slots. These were private constants, so the whole change is internal: tableSlot() takes the enum, the generic integer-keyed helpers (requireEntry/addPointerEntry/ addLongEntry, which also serve plain payload indexes) take ->value. BC: every public entry point keeps working unchanged. ArgumentEntry::SEND_* and getSendMode(): int and ModuleDependency::MODULE_* and getDependencyType(): int / getRelation(): ?string are kept as @deprecated scalar aliases delegating to the enums, so the existing tests pass untouched; the constructor and factories accept enum cases and the legacy scalars alike. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
OpLine::getValuePointer() mapped one operand type to one pointer expression through a switch with a pre-seeded $pointer; match says the same thing without the mutable accumulator and makes the unhandled-type branch a first-class arm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
CacheMetaInfo carries six fields, four of which are plain ints, and rebuilds itself positionally in three with*() methods plus two named constructors. Transposing memSize with strSize (or timestamp with checksum) there compiles, runs and writes a silently corrupt file-cache header - the failure surfaces much later as a cache miss or a wrong payload, never at the call site. Every reconstruction now passes its arguments by name, which makes such a transposition impossible to write. The fields themselves become public readonly, so the header can also be read as the plain value object it is; the existing accessor methods stay as thin delegates to the identically named properties, keeping the reading API (and its tests) untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
copy() never copied anything: it mirrors zend_string_copy(), which shares the existing string by taking one more reference (and takes none for an immortal interned string), then hands the very same entry back. The name invited call sites to believe they held an independent string, which for engine memory is the kind of misunderstanding that ends in a double release. addReference() states what happens. It is a genuinely new operation rather than a synonym of the existing incrementReferenceCount() primitive: that one refuses immutable payloads outright, while this one applies the engine's interned-is-immortal rule and returns $this for chaining. copy() stays as a delegate carrying PHP 8.4's #[\Deprecated] attribute, so existing consumers keep working and hear about the replacement. The three internal callers (FunctionBodySwap's two bucket-ownership paths and the adopted class-constant doc comment) are migrated, so nothing inside the library triggers the deprecation - which matters because the test suite runs with failOnDeprecation="true". No test calls copy(), so the runtime attribute could be used instead of a docblock-only deprecation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Closure::fromCallable([$this, 'handle']) is the pre-8.1 spelling of $this->handle(...); the new form names the method statically, so a rename is caught by the compiler instead of failing at install time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
- ReflectionClass::getMagicSlotFor() is an array_find over the magic slot field names: the loop already returned the matching field name. - HotSwap::assertSourceDeclaresClass() is an array_any over the collected declaration names. - ReflectionMethod::equals() is an array_all over the op_array body metrics. All three keep the original short-circuit semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
- strpos($name, 'prefix') === 0 becomes str_starts_with() in ReflectionExtension::__debugInfo() - get_class($object) becomes $object::class in the two FFI-boundary warning paths and in the hook publication-board lookup - $systemId = $systemId ?? ... becomes $systemId ??= ... in BinaryCacheFile::locate() ReflectionClass::__construct() keeps get_class(): its $classNameOrObject parameter is untyped, so ::class cannot be resolved at level max there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…-6pfu7y-quality-debt chore(quality): type the test suite, analyse the root files, enforce the license header, add phpstan-phpunit
…ization-6pfu7y-syntax-84
…-6pfu7y-api-84 feat: adopt PHP 8.4 API surface — asymmetric visibility, enums, named arguments, #[\Deprecated]
Core, Compiler, ExecutionData-adjacent AST nodes and ReflectionValue get the same pass as the rest of the tree: #[\Override] on interface implementations, the node-factory dispatch as a match, typed-constant and string-idiom modernization. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Typed constants meet the enum migration: ModuleDependency keeps #216's deprecated int aliases with this branch's const types; the PersistentHeap SLOT_* constants are gone entirely - the DescriptorSlot enum supersedes them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…-6pfu7y-syntax-84 refactor: PHP 8.4-era syntax modernization pass
Carries PRs #203, #204, #205, #212, #213, #216 and #217 to the 8.5 line. All five conflicts are the same shape - the 8.5-specific constant lists (NodeKind AST kinds, Core class flags, PayloadRelocator AST walk cases, OpCode opcodes) and composer.json dev requirements against the 8.4 line's const-int typing - resolved by keeping master's values and membership with the typing applied, and merging the dev requirements (phpunit ^12.2 || ^13.0 stays, phpstan-phpunit arrives, sorted). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
The ConstantNames migration (8.4) obsoleted the private name caches that only master's side of NodeKind/OpCode still declared - deleted. The 8.5-only $this-swap test asserts through a copy now: phpstan-phpunit (new on the 8.4 line) narrows an asserted $this to never, which the engine-level swap invalidates underneath the analyser. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
performance.yml is new on the 8.4 line, so the cascade carried it over verbatim with PHP_MINOR '8.4' - no textual conflict flagged the version-specific line. On master the benchmark must run the 8.5 interpreter or the version guard refuses to boot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
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
2 participants
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.
Carries the remainder of the modernization campaign — #203 (reflection dedup), #204 (memory ownership), #205 (hook consolidation), #212 (test gaps), #213 (quality debt), #216 (8.4 API surface) and #217 (syntax pass) — to the 8.5 line. Supersedes the automation's conflicted #215, which can be closed once this merges.
Conflict resolutions (5 files, all the same shape)
Master's 8.5-specific constant lists vs the 8.4 line's new
const inttyping — resolved by keeping master's values and membership with the typing applied:src/AbstractSyntaxTree/NodeKind.php— 8.5 AST kind values (incl.AST_OP_ARRAY,AST_PROPERTY_HOOK), typedsrc/Core.php— 8.5 class-flag values, typedsrc/OpCache/PayloadRelocator.php— 8.5 AST-walk special cases (ZEND_AST_OP_ARRAY,ZEND_AST_CALLABLE_CONVERT), typedsrc/System/OpCode.php— 8.5 opcode list (incl.DECLARE_ATTRIBUTED_CONST), typedcomposer.json— dev requirements merged:phpunit ^12.2 || ^13.0stays (8.5 line),phpstan/phpstan-phpunitarrives (8.4 line), sortedCross-branch drift settled (follow-up commit)
NodeKind::$constantNames/OpCode::$opCodeNames— dead since the 8.4 line'sConstantNamesmigration; only master's side still declared them. Deleted.ExecutionDataTest::testGetThis()(8.5-only$this-swap block) — asserts through a copy now:phpstan-phpunit(new via chore(quality): type the test suite, analyse the root files, enforce the license header, add phpstan-phpunit #213) narrows an asserted$thistonever, which the engine-level This-slot swap invalidates underneath the analyser.Validation
Container PHP 8.5.9 NTS matches master's target, so beyond the static gates the real suite ran locally:
vendor/bin/phpstan analyse(level max, phpVersion 80500, phpstan-phpunit active) — cleanphp-cs-fixer— cleanEngineConstantsTest,EngineLayoutTest) pass — direct verification of the constant-list resolutions above. 8 tests report problems in this sandbox: the identical set was previously proven pre-existing on pristineorigin/masterin this same environment during the merge: cascade 8.4 into master (modernization wave, conflicts resolved) #214 cascade (order-sensitive suite state, documented there). CI on the real legs is the gate.🤖 Generated with Claude Code
https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Generated by Claude Code