Skip to content

feat(objects): a keyed array literal is a valid default for a nullable-array property - #1053

Open
Guikingone wants to merge 2 commits into
mainfrom
fix/688-union-array-default
Open

Guikingone wants to merge 2 commits into
mainfrom
fix/688-union-array-default

Conversation

@Guikingone

Copy link
Copy Markdown
Collaborator

Fixes #688.

Declaring the class was enough to be refused -- no read, no write, no iteration:

class C { public ?array $x = ["k" => 1]; }
EIR backend error: unsupported EIR backend feature:
object_new for default value of property $x with PHP type Union([Array(Mixed), Void])

What was left

Most of #688 had already been fixed: a mixed/union slot learned to box an INDEXED
array literal, which covers the issue's three reported rows -- ?array $x = [1, 2],
?array $x = [], and mixed $x = [1, 2]. The KEYED spelling had no default form at
all and still failed with the same message.

PHP has no separate associative array type, so ["k" => 1] in a ?array slot is the
same default as [1, 2] is; only the storage a string key needs differs.
LiteralDefaultValue::BoxedAssocArray is the keyed sibling of the existing
BoxedArray, built from the classifier's existing AssocArray arm and the boxing
step BoxedArray already performs.

Both emitters use the OWNED boxer, for the reason recorded on BoxedArray: the
literal allocated the container and the box takes its own reference, so the plain
boxer retains without releasing and leaks one block per object.

Verified

Eleven shapes against host PHP 8.5.10, all byte-identical: the keyed and positional
spellings on ?array, the empty literal, mixed holding a keyed and a positional
literal, array|string, a static nullable-array property, and -- beside them -- the
shapes that already worked, so a future change cannot fix one and lose the other: a
plain array slot, a mixed slot holding a scalar, an explicit null default, and
the constructor-assignment workaround the issue documented.

The classifier arm was confirmed load-bearing by disabling it: both tests fail with
the original object_new for default value of property $x with PHP type Union([Array(Mixed), Void]).

--gc-stats reports allocs=2000 frees=2000 over 200 iterations, and a heap-debug
test pins a clean summary across the keyed, mixed-keyed and positional defaults
together.

cargo test --test codegen_tests -- codegen::objects passes in full (256), as do the
1680 lib tests and 1534 error tests.

One shape is still refused, and it is not this one

A default whose ELEMENTS are array literals -- public array $x = [[1], [2]]; -- is
refused, and a plain array property fails exactly as a ?array one does, so it is
not about the slot at all. LiteralArrayElement has no nested-container form. Filed
as #1052; the same literal is accepted as a local, a parameter default and a class
constant.

Docs and example

docs/php/classes.md states that nullable and union array properties take either
literal spelling, and states the nested-element limitation with its workaround rather
than leaving it to be discovered.

examples/typed-properties/main.php gains a Request whose ?array defaults use
both spellings beside a mixed one and a null one, and clears one to null to show
the slot still holds it. The example is pinned by
test_example_typed_properties_compiles_and_runs, whose expectation is updated in the
same commit and is verbatim host PHP 8.5.10.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr

@github-actions github-actions Bot added area:codegen Touches target-aware assembly or backend lowering. size:s Small pull request. type:feature Introduces new user-visible behavior or capabilities. labels Sep 16, 2026
@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the implementation consistently handles keyed defaults across instance and static paths and includes focused ownership coverage.

Summary

This PR adds support for keyed associative-array literal defaults on nullable, mixed, and other union-typed properties.

  • Introduces a boxed associative-array default representation and emits it for instance and static properties.
  • Uses owned boxing to preserve correct reference-count ownership.
  • Adds behavioral, example, and heap-debug regression coverage.
  • Documents supported array-default forms and the remaining nested-array limitation.
  • Moves the ownership regression into the required runtime GC test area, fully addressing the previous review finding.

Reviews (4) · Last reviewed commit: "test(runtime_gc): file the boxed-default..."

Comment thread tests/codegen/objects/untyped_property_defaults.rs Outdated
@Guikingone Guikingone self-assigned this Sep 16, 2026
@Guikingone
Guikingone requested a review from nahime0 September 16, 2026 16:44
@Guikingone
Guikingone force-pushed the fix/688-union-array-default branch from 3bf0552 to d91755b Compare September 16, 2026 20:59
…e-array property

Declaring the class was enough to be refused -- no read, no write, no iteration:

    class C { public ?array $x = ["k" => 1]; }
    EIR backend error: unsupported EIR backend feature:
    object_new for default value of property $x with PHP type Union([Array(Mixed), Void])

Most of #688 had already been fixed: a `mixed`/union slot learned to box an INDEXED
array literal, which covers the issue's three reported rows -- `?array $x = [1, 2]`,
`?array $x = []`, and `mixed $x = [1, 2]`. The KEYED spelling had no default form at
all and still failed with the same message.

PHP has no separate associative array type, so `["k" => 1]` in a `?array` slot is the
same default as `[1, 2]` is; only the storage a string key needs differs.
`LiteralDefaultValue::BoxedAssocArray` is the keyed sibling of the existing
`BoxedArray`, built from the classifier's existing `AssocArray` arm and the boxing
step `BoxedArray` already performs.

Both emitters use the OWNED boxer, for the reason recorded on `BoxedArray`: the
literal allocated the container and the box takes its own reference, so the plain
boxer retains without releasing and leaks one block per object.

Eleven shapes against host PHP 8.5.10, all byte-identical: the keyed and positional
spellings on `?array`, the empty literal, `mixed` holding a keyed and a positional
literal, `array|string`, a static nullable-array property, and -- beside them -- the
shapes that already worked, so a future change cannot fix one and lose the other: a
plain `array` slot, a `mixed` slot holding a scalar, an explicit `null` default, and
the constructor-assignment workaround the issue documented.

The classifier arm was confirmed load-bearing by disabling it: both tests fail with
the original `object_new for default value of property $x with PHP type
Union([Array(Mixed), Void])`.

`--gc-stats` reports `allocs=2000 frees=2000` over 200 iterations, and a heap-debug
test pins a clean summary across the keyed, `mixed`-keyed and positional defaults
together.

`cargo test --test codegen_tests -- codegen::objects` passes in full (256), as do the
1680 lib tests and 1534 error tests.

A default whose ELEMENTS are array literals -- `public array $x = [[1], [2]];` -- is
refused, and a plain `array` property fails exactly as a `?array` one does, so it is
not about the slot at all. `LiteralArrayElement` has no nested-container form. Filed
as #1052; the same literal is accepted as a local, a parameter default and a class
constant.

`docs/php/classes.md` states that nullable and union array properties take either
literal spelling, and states the nested-element limitation with its workaround rather
than leaving it to be discovered.

`examples/typed-properties/main.php` gains a `Request` whose `?array` defaults use
both spellings beside a `mixed` one and a `null` one, and clears one to null to show
the slot still holds it. The example is pinned by
`test_example_typed_properties_compiles_and_runs`, whose expectation is updated in the
same commit and is verbatim host PHP 8.5.10.

Fixes #688

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
…ts live

AGENTS.md is explicit: ownership, aliasing, cycles, heap debug, stack args and COW
tests go in `tests/codegen/runtime_gc/`. This one sat under `tests/codegen/objects/`
because that is where the behaviour it came from is tested, which is the wrong axis --
someone auditing what the heap-debug suite covers would not have found it.

It moves to `runtime_gc/boxed_property_defaults.rs` rather than into
`runtime_gc/regressions.rs`, which is already 4592 lines against the 500-line
guideline. The new file states the invariant it exists for: the literal allocates the
container, the box takes its own reference, so the OWNED boxer is required, and the
loop is what makes the difference visible -- the plain boxer leaks exactly one block
per object, which a single construction hides.

The behavioural coverage stays where it belongs.
`test_array_literal_defaults_on_union_and_mixed_properties` -- the eleven-shape fixture
pinning what the defaults PRINT -- is untouched in
`tests/codegen/objects/untyped_property_defaults.rs`.

Both pass: 15 tests across the two modules.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
@Guikingone
Guikingone force-pushed the fix/688-union-array-default branch from d91755b to b495e81 Compare September 18, 2026 13:17
@Guikingone

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main; this branch is up to date and mergeable again.

Heads-up on an overlap found while rebasing. This PR and #1069 both add the same
LiteralDefaultValue::BoxedAssocArray { value_type, entries } variant to
src/codegen/literal_defaults.rs, arrived at independently:

They are the same addition, so whichever merges first, the other will conflict on that variant
and on its two match arms (block_emit.rs, objects/property_defaults.rs). The resolution is
to keep one copy of the variant, not both — I will rebase the second one onto the first once
the order is decided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:codegen Touches target-aware assembly or backend lowering. size:s Small pull request. type:feature Introduces new user-visible behavior or capabilities.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An array literal default on a ?array or mixed property fails to compile

1 participant