compiler

1826 Commits

2 Branches

17 Tags

66 MiB

Tag: Branch: Tree: e088776d24

Commit Graph

Author	SHA1	Message	Date
Alessio Giacobbe	4ca9072f36	fix(codegen): register enum-case class constants as real case objects (#51 ) --skip-tests * fix(gen_stub): register enum-case class constants as persistent AST constants A class constant valued by an enum case was registered with the folded scalar (the backing value, or the case name for pure cases), so constant('K::CB'), $cls::CB and reflection observed an int/string where PHP has the case object, and K::CB === E::B was false on every dynamic path. Enum case objects have request lifetime and can never sit in the persistent class-entry tables, in any request-init rebinding scheme least of all: writing a request-owned object into the shared table is unsafe under concurrent ZTS requests. Reuse the engine's own mechanism for internal enums instead: the constant is declared as a persistent IS_CONSTANT_AST holding the Enum::Case fetch, so Zend separates the class constants table into request-local mutable storage on first access, evaluates the fetch there, and cleans it up at request shutdown. Identity is preserved for static access, constant(), dynamic class access and reflection, with no module-lifecycle hooks and no registration-order sensitivity. Case identity flows through compile-time constant evaluation as an EnumCaseRef value instead of a scalar, so it also survives constant expressions (true ? E::A : E::B), constant chains, typed class constants (declared type and AST value are registered together), and internal enum cases such as RoundingMode::HalfEven, which previously aborted stub generation. The runtime expression path (php::getEnumCase) is unchanged. The preprocessor also no longer reads the raw ->value property off arbitrary case expressions (`case A = 1 + 1;` warned and was recorded as a pure case): only literal backing values are recorded eagerly, and no compile-time consumer needs the evaluated scalar - gen_stub evaluates the registration value from the AST itself. * fix(gen_stub): constrain enum-case AST registration to class constants The persistent IS_CONSTANT_AST representation leaked into property and parameter defaults, whose persistent tables reject refcounted zvals: startup died with "Internal zvals cannot be refcounted". EvaluatedValue now carries the case identity in a dedicated field while its value degrades to what those consumers read before case identity existed (the host case object for internal enums, the literal backing value or case name for compiled ones), and only class-constant registration opts into the AST. Property/parameter defaults keep flowing through their existing runtime-restore machinery unchanged. Also parenthesize a folded constant operand before appending a member access: the C++ ternary of `const VALUE = cond ? E::A : E::B;` bound `.attr("value")` to its else branch only, so `K::VALUE->value` evaluated to the case object instead of its backing value. * fix(gen_stub): give persistent AST constants a complete teardown lifecycle destroy_zend_class() asserts (in debug builds) that every persistent AST constant remaining on an internal class is CONST_ENUM_INIT, and its teardown frees only the allocation referenced by Z_AST — the previous representation left a CLASS_CONST root behind (assertion failure at shutdown on 8.4/8.5 debug builds) and leaked the two separately allocated children. The AST is now built in one contiguous persistent allocation (ast_ref, root, both zval children — mirroring Zend's own persistent enum AST builder), and every generated file with AST constants emits a release function that runs from the module's MSHUTDOWN, before Zend's class teardown: it frees the single block and restores the constant slot to null, so destroy_zend_class() never sees a foreign AST. Request-local mutable copies are unaffected (no request is live at MSHUTDOWN). CONST_ENUM_INIT itself is not usable here: that node constructs a new case object rather than fetching the canonical registered one, which would break case identity again. * fix(codegen): reject lifecycles that cannot release AST constants The typephp_release_ast_constants_() teardown ran only from MSHUTDOWN, which is not a general pre-class-destruction hook: for a MODULE_TEMPORARY module loaded through dl(), module_destructor() runs clean_module_classes() before the shutdown callback, so the foreign ZEND_AST_CLASS_CONST reached destroy_zend_class() first and still tripped the debug assertion; and a MINIT that fails after registering such a constant never sets module_started, so MSHUTDOWN is not guaranteed to run at all. The generated module now enforces the lifecycle contract instead of assuming it. When the module declares any enum-case AST constant, MINIT opens with a guard that rejects MODULE_TEMPORARY (zend_error E_WARNING, return FAILURE) before a single class is registered — with nothing in the class table, teardown is trivially safe. MINIT is also restructured so that every step that can return FAILURE precedes the first register_class_() call: the AST constants are installed by the infallible tail (class registration, then symbol registration), so a FAILURE return can never leave a foreign AST in the persistent tables. The generator itself throws if a future change introduces a FAILURE return after registration begins. The MSHUTDOWN release is unchanged and remains the supported, persistent-module path. EnumCaseAstConstantLifecycleTest asserts the guard exists exactly when AST constants exist, that it precedes every registration step, that no FAILURE return follows the first class registration, and that MSHUTDOWN releases the constants before any other teardown. The enum-case phpt gains a never-accessed constant so the full process shutdown it already performs also covers a pristine persistent AST; a dl()-path phpt is not feasible because the harness only builds standalone binaries whose module is registered persistently (documented in the test file).	1 month ago

Author

SHA1

Message

Date

Alessio Giacobbe

4ca9072f36

fix(codegen): register enum-case class constants as real case objects (#51 ) --skip-tests

* fix(gen_stub): register enum-case class constants as persistent AST constants

A class constant valued by an enum case was registered with the folded
scalar (the backing value, or the case name for pure cases), so
constant('K::CB'), $cls::CB and reflection observed an int/string where
PHP has the case object, and K::CB === E::B was false on every dynamic
path. Enum case objects have request lifetime and can never sit in the
persistent class-entry tables, in any request-init rebinding scheme
least of all: writing a request-owned object into the shared table is
unsafe under concurrent ZTS requests.

Reuse the engine's own mechanism for internal enums instead: the
constant is declared as a persistent IS_CONSTANT_AST holding the
Enum::Case fetch, so Zend separates the class constants table into
request-local mutable storage on first access, evaluates the fetch
there, and cleans it up at request shutdown. Identity is preserved for
static access, constant(), dynamic class access and reflection, with no
module-lifecycle hooks and no registration-order sensitivity.

Case identity flows through compile-time constant evaluation as an
EnumCaseRef value instead of a scalar, so it also survives constant
expressions (true ? E::A : E::B), constant chains, typed class
constants (declared type and AST value are registered together), and
internal enum cases such as RoundingMode::HalfEven, which previously
aborted stub generation. The runtime expression path (php::getEnumCase)
is unchanged.

The preprocessor also no longer reads the raw ->value property off
arbitrary case expressions (`case A = 1 + 1;` warned and was recorded
as a pure case): only literal backing values are recorded eagerly, and
no compile-time consumer needs the evaluated scalar - gen_stub
evaluates the registration value from the AST itself.

* fix(gen_stub): constrain enum-case AST registration to class constants

The persistent IS_CONSTANT_AST representation leaked into property and
parameter defaults, whose persistent tables reject refcounted zvals:
startup died with "Internal zvals cannot be refcounted". EvaluatedValue
now carries the case identity in a dedicated field while its value
degrades to what those consumers read before case identity existed
(the host case object for internal enums, the literal backing value or
case name for compiled ones), and only class-constant registration opts
into the AST. Property/parameter defaults keep flowing through their
existing runtime-restore machinery unchanged.

Also parenthesize a folded constant operand before appending a member
access: the C++ ternary of `const VALUE = cond ? E::A : E::B;` bound
`.attr("value")` to its else branch only, so `K::VALUE->value`
evaluated to the case object instead of its backing value.

* fix(gen_stub): give persistent AST constants a complete teardown lifecycle

destroy_zend_class() asserts (in debug builds) that every persistent
AST constant remaining on an internal class is CONST_ENUM_INIT, and its
teardown frees only the allocation referenced by Z_AST — the previous
representation left a CLASS_CONST root behind (assertion failure at
shutdown on 8.4/8.5 debug builds) and leaked the two separately
allocated children.

The AST is now built in one contiguous persistent allocation (ast_ref,
root, both zval children — mirroring Zend's own persistent enum AST
builder), and every generated file with AST constants emits a release
function that runs from the module's MSHUTDOWN, before Zend's class
teardown: it frees the single block and restores the constant slot to
null, so destroy_zend_class() never sees a foreign AST. Request-local
mutable copies are unaffected (no request is live at MSHUTDOWN).

CONST_ENUM_INIT itself is not usable here: that node constructs a new
case object rather than fetching the canonical registered one, which
would break case identity again.

* fix(codegen): reject lifecycles that cannot release AST constants

The typephp_release_ast_constants_*() teardown ran only from MSHUTDOWN,
which is not a general pre-class-destruction hook: for a
MODULE_TEMPORARY module loaded through dl(), module_destructor() runs
clean_module_classes() before the shutdown callback, so the foreign
ZEND_AST_CLASS_CONST reached destroy_zend_class() first and still
tripped the debug assertion; and a MINIT that fails after registering
such a constant never sets module_started, so MSHUTDOWN is not
guaranteed to run at all.

The generated module now enforces the lifecycle contract instead of
assuming it. When the module declares any enum-case AST constant, MINIT
opens with a guard that rejects MODULE_TEMPORARY (zend_error E_WARNING,
return FAILURE) before a single class is registered — with nothing in
the class table, teardown is trivially safe. MINIT is also restructured
so that every step that can return FAILURE precedes the first
register_class_*() call: the AST constants are installed by the
infallible tail (class registration, then symbol registration), so a
FAILURE return can never leave a foreign AST in the persistent tables.
The generator itself throws if a future change introduces a FAILURE
return after registration begins. The MSHUTDOWN release is unchanged
and remains the supported, persistent-module path.

EnumCaseAstConstantLifecycleTest asserts the guard exists exactly when
AST constants exist, that it precedes every registration step, that no
FAILURE return follows the first class registration, and that MSHUTDOWN
releases the constants before any other teardown. The enum-case phpt
gains a never-accessed constant so the full process shutdown it already
performs also covers a pristine persistent AST; a dl()-path phpt is not
feasible because the harness only builds standalone binaries whose
module is registered persistently (documented in the test file).

1 month ago

1 Commits (e088776d24c6e3e66f58ce56ce2ffea3d127c769)