Skip to content

fix(reflection): correct _IS_BOOL/_IS_NUMBER for PHP 8.1+ and add named cast-type enums - #155

Merged
lisachenko merged 1 commit into
8.4from
claude/fix-cast-type-ids
Aug 7, 2026
Merged

fix(reflection): correct _IS_BOOL/_IS_NUMBER for PHP 8.1+ and add named cast-type enums#155
lisachenko merged 1 commit into
8.4from
claude/fix-cast-type-ids

Conversation

@lisachenko

Copy link
Copy Markdown
Owner

Fixes #152.

The bug

PHP 8.1 inserted IS_NEVER = 17 into the zval type table (Zend/zend_types.h), shifting _IS_BOOL to 18 and _IS_NUMBER to 19 — but ReflectionValue still declared the pre-8.1 values. The drift was worse than a miss: the engine passes 18 for every boolean cast, which the stale table resolves to _IS_NUMBER, so a cast handler dispatching on the documented constants misroutes (bool) into its numeric branch silently.

The generated ground truth in include/8.4/*/constants.php has carried the correct values all along — ReflectionValue simply wasn't covered by EngineConstantsTest. It is now, for both the IS_* and _IS_* prefixes, so this class of drift fails CI in the future.

The API addition

Because namesake integer constants can drift silently between minors, the hooks gain a named, ground-truth-guarded API (as discussed in #152):

  • CastType (int-backed enum): Long, Double, String, Array, Object, Bool, Number — surfaced via CastObjectHook::getCastTypeEnum(): ?CastType. Case values are asserted against the generated constants in EngineConstantsTest.
  • PropertyPurpose (int-backed enum): Debug, ArrayCast, Serialize, VarExport, Json, GetObjectVars — surfaced via GetPropertiesForHook::getPurposeEnum(): ?PropertyPurpose. zend_prop_purpose is not exported by the generator manifest yet, so this one carries no generated guard — adding ZEND_PROP_PURPOSE_* to tools/generator/symbols.php (with a header regeneration) would close that gap as a follow-up.

Both getCastType(): int / getPurpose(): int remain untouched for BC.

The regression test

testInstallCastObjectHandler previously asserted (bool)false and passed by accident: the bool cast (18) fell into the _IS_NUMBER branch returning long(1), and the engine reports any non-IS_TRUE retval of a boolean cast as false. The handler now dispatches on the enum and asserts (bool)true, which only the Bool branch can produce — the assertion fails on the stale ids and passes on the corrected ones.

Verified: full suite green on PHP 8.4.19 NTS (411 tests), PHPStan level max clean, PER-CS2.0 clean.

Note for merge-up: master (8.5) declares the same stale values, so the cascade applies cleanly; the 8.0 branch is untouched and correct (IS_NEVER did not exist there).

Downstream context: found implementing casts in lisachenko/native-php-matrix#17, which carries a local-constant workaround that can be dropped once this ships in a tagged release.


🤖 Generated with Claude Code

https://claude.ai/code/session_01JsbdqisRfGuujN3QD9n8En


Generated by Claude Code

…ed cast-type enums

PHP 8.1 inserted IS_NEVER = 17 into the zval type table, shifting
_IS_BOOL to 18 and _IS_NUMBER to 19, but ReflectionValue still declared
the pre-8.1 values. The drift was invisible and worse than a miss: the
engine passes 18 for every boolean cast, which the stale table resolves
to _IS_NUMBER, so cast handlers dispatching on the documented constants
misroute (bool) into their numeric branch. The generated ground truth in
include/8.4/*/constants.php has carried the correct values all along —
ReflectionValue was simply not covered by EngineConstantsTest, so the
drift went unnoticed. It is covered now, for both the IS_* and _IS_*
prefixes.

Because namesake integer constants can drift silently between minors,
the hooks also gain a named, ground-truth-guarded API:

- CastType (backed enum): Long, Double, String, Array, Object, Bool,
  Number — with CastObjectHook::getCastTypeEnum()
- PropertyPurpose (backed enum): Debug, ArrayCast, Serialize, VarExport,
  Json, GetObjectVars — with GetPropertiesForHook::getPurposeEnum()

CastType case values are asserted against the generated constants in
EngineConstantsTest; PropertyPurpose mirrors zend_prop_purpose, which
the generator manifest does not export yet.

The cast-handler test now dispatches on the enum and asserts that
(bool) yields true: the engine only accepts IS_TRUE/IS_FALSE as a
boolean cast result, so a handler misrouted into the numeric branch
produces long(1), which the engine reports as false — the assertion
fails on the stale ids and passes on the corrected ones.

Fixes #152

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JsbdqisRfGuujN3QD9n8En
@lisachenko
lisachenko marked this pull request as ready for review August 7, 2026 22:02
@lisachenko
lisachenko merged commit 0725889 into 8.4 Aug 7, 2026
7 checks passed
@lisachenko
lisachenko deleted the claude/fix-cast-type-ids branch August 7, 2026 22:02
lisachenko pushed a commit to lisachenko/native-php-matrix that referenced this pull request Aug 7, 2026
…-through

z-engine 8.4.1 (PRs lisachenko/z-engine#155 and #156) ships the API the
local workarounds stood in for: CastType/PropertyPurpose enums exposed
via getCastTypeEnum()/getPurposeEnum(), guarded upstream against the
generated engine ground truth, and a CastObjectHook whose fall-through
behaves exactly like an uninstalled handler. Drop the local
ENGINE_IS_BOOL/ENGINE_IS_NUMBER/PROP_PURPOSE_* constants — values that
can silently drift between PHP minors — and dispatch on the named cases
instead.

The numeric-cast fallback is gone entirely: __cast now defers to
proceed()/getResult(), and the failed cast propagates to the engine
caller, which emits its own "could not be converted to int/float"
warning and substitutes 1 — the previously undeliverable default
diagnostic is restored, and the fallback test asserts it.

Raises the z-engine floor to ~8.4.1 accordingly, in lockstep with the
PHP 8.4 pin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JsbdqisRfGuujN3QD9n8En
lisachenko pushed a commit to lisachenko/native-php-matrix that referenced this pull request Aug 7, 2026
…-through

z-engine 8.4.1 (PRs lisachenko/z-engine#155 and #156) ships the API the
local workarounds stood in for: CastType/PropertyPurpose enums exposed
via getCastTypeEnum()/getPurposeEnum(), guarded upstream against the
generated engine ground truth, and a CastObjectHook whose fall-through
behaves exactly like an uninstalled handler. Drop the local
ENGINE_IS_BOOL/ENGINE_IS_NUMBER/PROP_PURPOSE_* constants — values that
can silently drift between PHP minors — and dispatch on the named cases
instead.

The numeric-cast fallback is gone entirely: __cast now defers to
proceed()/getResult(), and the failed cast propagates to the engine
caller, which emits its own "could not be converted to int/float"
warning and substitutes 1 — the previously undeliverable default
diagnostic is restored, and the fallback test asserts it.

Raises the z-engine floor to ~8.4.1 accordingly, in lockstep with the
PHP 8.4 pin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JsbdqisRfGuujN3QD9n8En
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(reflection): _IS_BOOL/_IS_NUMBER constants predate PHP 8.1's IS_NEVER, so boolean casts dispatch on the wrong type id

2 participants