[nahime0]

Implements simplexml_load_string, simplexml_load_file, simplexml_import_dom, SimpleXMLElement, and SimpleXMLIterator through the XML bridge and managed libxml2.

AOT and opaque eval share live document views with DOM imports. Regression coverage includes child and attribute writes, dimensions, namespaces, XPath, conversions, operators, serialization, iteration, stream loading, subclass hooks and ownership. Examples and generated builtin documentation describe the public surface across all five supported targets.

Native factories follow PHP by permitting abstract subclasses and bypassing constructors. Failed wrapper allocation releases its native handle. Abstract method calls raise the correct catchable Error before argument effects, including runtime method names and nullsafe calls. Inaccessible protected methods retain inherited __call. Strict constructorless Reflection allocation refuses abstract classes. Factory and dispatch regressions retain both optimizer modes and strict default-heap checks, with emitter coverage on every target.

The argument-bound correction preserves PHP argument errors before parsing or file I/O. Both load functions reject options outside the signed C-int range with ValueError. The constructor uses Exception with PHP's argument message, outside its parse-error catch. String and namespace length checks follow the corresponding PHP entry points. Native fail-closed checks remain. One new AOT/opaque-eval bundle checks both overflow signs, PHP_INT_MAX/MIN, exact messages, absence of warnings, valid boundary values, both optimizer modes and strict heap cleanup.

Signed head 984e7222c85ccb4071c22fee13c3dd94525691ac includes main 34cbe933841275567f61af3731667ad2da3b88a3. Necessary integration preserves upstream fixes and tests. General native-reference cleanup and eager full Reflection metadata retention remain separate.

Recovered documents without a root retain their document handle. Node-dependent operations raise PHP's catchable initialization Error; scalar casts, iterator controls and valid empty selections retain their distinct behavior. The original initialization regression passed on all three CI hosts after the hidden dimension-argument collector gained its missing unwind owner.

The latest correction addresses the two findings from the completed Grok review of the preceding head. Discarded Bool/Array casts and truthiness retain native conversion errors through the existing capability-gated effect and throw summaries. Empty or identical if branches preserve the implicit Bool conversion. Computed dimension keys reuse existing string claims and caller owner pins, with normal and exceptional cleanup balanced for fresh strings, concat, Mixed and normalized casts. No ABI, native-reference layout or general runtime/optimizer redesign is included.

The same initialization bundle retains all 25 original Error checks and expands to 47 paths on five invalid objects, four valid empty/root controls and four computed-key success cases. AOT and opaque eval, both optimizer modes, exact output and strict default-heap checks remain. Existing regressions, including the 200,000-node case, worker counts, runtime limits and deadlines are preserved.

Logical xor now invokes the same native boolean dispatcher as conditions. Both operands are evaluated before either native conversion, with existing capture/borrow/pin helpers preserving snapshots and cleanup. Boolean results remain canonical, and the path without native boolean handlers is preserved. The regression retains the failing XOR assertions and adds a fresh RHS side effect before the initialization Error, temporary cleanup and valid native/numeric XOR results. The follow-up wraps existing pinned value IDs in their typed LoweredValue descriptors before truthiness lowering; this corrects a Rust type error in one file and leaves all tests unchanged.

The discarded expression statements use redundant grouping parentheses accepted by the existing parser. The fixture correction changes no production code, cases, assertions or expected output; both stock-PHP outputs remain identical. General support for ungrouped unary/literal expression statements is tracked separately.

Validation on this exact corrected head is in progress:

CI, SDK, PDO live, CodeQL.

The preceding CI fully terminated before this push. Its 77 jobs across CI, SDK, PDO and CodeQL ended with 11 successes, 38 failures and 28 skips. All 38 nonpass logs were inspected: 35 report the same Rust E0308 type mismatch and three are dependent aggregates. Compilation stopped before runtime tests, so those runs provide no runtime validation. CodeQL succeeded. No manual retry was used. The corrected head must pass the full CI and both optimizer modes before readiness is claimed.

Local validation remains static syntax, tiny independent stock-PHP oracles and Git hygiene. Both expanded fixture variants match PHP 8.5.12. Project builds, tests, exporters and audits run only in CI. After all three builds and focused regressions pass on this corrected head, a new independent Grok review is required. No previous verdict transfers.

Greptile is waived by the maintainer. The PR remains open and outside the merge queue. The requested endpoint is successful exact-head CI and a clean independent Grok review, followed by notification and a stop. Do not enqueue or merge.

[greptile-apps]

Confidence Score: 1/5

[High impact] The PR reaches a new XML feature and shared compiler and runtime paths.

Do not merge yet: the codegen tests cannot compile, generator loops leak references, and both numbered previous findings remain unresolved.

Findings

1.  Codegen tests cannot compile ▶
2.  Generator loops leak yielded cells ▶
3.  Scalar probes fail ▶
4.  Appends replace static values ▶
5.  Assembly comments miss required format ▶

Fix with agent prompt

### Issue 1
tests/codegen/io/streams.rs:8287-8290
The new Rust test sits inside the PHP raw string in `test_stream_get_filters_includes_registered_patterns`. Its closing `"#` ends that existing string, leaving `stream_filter_register(...)` and PHP variables outside any string or function. The codegen test binary cannot compile, and the new test is not registered as a Rust test.

Move the complete new test after the existing test's closing brace.

### Issue 2
src/codegen/lower_inst/iterators.rs:1802-1807
Removing the `return_ty.codegen_repr() != PhpType::Mixed` guard sends `Generator` keys and values through `box_compiled_iterator_result`. Their runtime helpers already return an owned `Mixed` cell but do not set the compiled-return ownership marker.

On Linux x86_64 without heap debugging, their call to `__rt_incref` leaves `r11` at zero. The new check then treats the owned result as borrowed and retains it again, leaking a reference for each yielded key or value. Keep these already-owned runtime results out of the compiled-return check, or explicitly mark them owned.

### Issue 3
src/types/checker/inference/expr/array_access.rs:186-187
The checker now accepts scalar offset and property probes, but lowering still treats them as container reads. For example, `function probe(int $v): bool { return isset($v['x']); }` passes the raw integer to `__rt_mixed_array_get` as a cell pointer. Calling it with a nonzero value such as `$argc` can crash instead of returning `false`.

The related property changes have the same gap: `$v->x ?? 'fallback'` reaches `resolve_property_slot` and fails code generation for an integer receiver. Add scalar-aware probe lowering before allowing these types, while preserving any key-expression side effects.

### Issue 4
src/types/checker/stmt_check/assignments/static_properties.rs:undefined-97
Allowing `Mixed` static-property appends exposes a path that replaces the whole value. With `class C { public static mixed $a = []; } C::$a[] = 1;`, `lower_static_property_array_push` emits a two-operand `RuntimeCall`. The backend sends that to `lower_mixed_cell_runtime_assign`, which replaces the array with `1` rather than appending. A boxed `ArrayAccess` object also misses its write callback.

Route compiled `Mixed` static properties through `MixedArrayAppend` with the required copy and writeback.

### Issue 5
src/codegen/lower_inst/builtins/io/filter_dispatch.rs:45
The new assembly in `lower_dynamic_filter_attach` has no `// -- description --` group comments. The AArch64 branch on this line also puts `//` directly after the code without a space.

The contributor guide requires group comments and column-81 alignment, or exactly one separating space when the code is longer. Apply that format to the new instruction groups. This repository requirement must be satisfied before merging.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds SimpleXML through the managed libxml2 bridge, including shared DOM imports, live selections, writes, XPath, conversions, and iteration.

• PHP programs can load XML and work with it through SimpleXML objects.
• SimpleXML objects work with PHP property, offset, and conversion operations.
• SimpleXML loads supported stream URLs and can share documents with DOM.
• SimpleXML documentation and CI coverage describe and exercise the new XML surface.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    PHP["PHP SimpleXML calls"] --> Prelude["Shared PHP prelude"]
    Prelude --> Rust["Rust document and selection owners"]
    Rust --> Shim["Opaque C shim"]
    Shim --> Libxml["Managed libxml2"]
    DOM["DOM imports"] --> Prelude
    Eval["eval calls"] --> Prelude

Loading

_{Reviews (13) · Last reviewed commit: "fix(runtime): retain mixed iterator retu..." · Reviewed by Greptile}

[greptile-apps]

Scalar probes fail

The checker now accepts scalar offset and property probes, but lowering still treats them as container reads. For example, function probe(int $v): bool { return isset($v['x']); } passes the raw integer to __rt_mixed_array_get as a cell pointer. Calling it with a nonzero value such as $argc can crash instead of returning false.

The related property changes have the same gap: $v->x ?? 'fallback' reaches resolve_property_slot and fails code generation for an integer receiver. Add scalar-aware probe lowering before allowing these types, while preserving any key-expression side effects.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/types/checker/inference/expr/array_access.rs
Line: 186-187

Comment:
**Scalar probes fail**

The checker now accepts scalar offset and property probes, but lowering still treats them as container reads. For example, `function probe(int $v): bool { return isset($v['x']); }` passes the raw integer to `__rt_mixed_array_get` as a cell pointer. Calling it with a nonzero value such as `$argc` can crash instead of returning `false`.

The related property changes have the same gap: `$v->x ?? 'fallback'` reaches `resolve_property_slot` and fails code generation for an integer receiver. Add scalar-aware probe lowering before allowing these types, while preserving any key-expression side effects.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Appends replace static values

Allowing Mixed static-property appends exposes a path that replaces the whole value. With class C { public static mixed $a = []; } C::$a[] = 1;, lower_static_property_array_push emits a two-operand RuntimeCall. The backend sends that to lower_mixed_cell_runtime_assign, which replaces the array with 1 rather than appending. A boxed ArrayAccess object also misses its write callback.

Route compiled Mixed static properties through MixedArrayAppend with the required copy and writeback.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/types/checker/stmt_check/assignments/static_properties.rs
Line: 97

Comment:
**Appends replace static values**

Allowing `Mixed` static-property appends exposes a path that replaces the whole value. With `class C { public static mixed $a = []; } C::$a[] = 1;`, `lower_static_property_array_push` emits a two-operand `RuntimeCall`. The backend sends that to `lower_mixed_cell_runtime_assign`, which replaces the array with `1` rather than appending. A boxed `ArrayAccess` object also misses its write callback.

Route compiled `Mixed` static properties through `MixedArrayAppend` with the required copy and writeback.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Codegen tests cannot compile

The new Rust test sits inside the PHP raw string in test_stream_get_filters_includes_registered_patterns. Its closing "# ends that existing string, leaving stream_filter_register(...) and PHP variables outside any string or function. The codegen test binary cannot compile, and the new test is not registered as a Rust test.

Move the complete new test after the existing test's closing brace.

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/codegen/io/streams.rs
Line: 8287-8290

Comment:
**Codegen tests cannot compile**

The new Rust test sits inside the PHP raw string in `test_stream_get_filters_includes_registered_patterns`. Its closing `"#` ends that existing string, leaving `stream_filter_register(...)` and PHP variables outside any string or function. The codegen test binary cannot compile, and the new test is not registered as a Rust test.

Move the complete new test after the existing test's closing brace.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Generator loops leak yielded cells

Removing the return_ty.codegen_repr() != PhpType::Mixed guard sends Generator keys and values through box_compiled_iterator_result. Their runtime helpers already return an owned Mixed cell but do not set the compiled-return ownership marker.

On Linux x86_64 without heap debugging, their call to __rt_incref leaves r11 at zero. The new check then treats the owned result as borrowed and retains it again, leaking a reference for each yielded key or value. Keep these already-owned runtime results out of the compiled-return check, or explicitly mark them owned.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/codegen/lower_inst/iterators.rs
Line: 1802-1807

Comment:
**Generator loops leak yielded cells**

Removing the `return_ty.codegen_repr() != PhpType::Mixed` guard sends `Generator` keys and values through `box_compiled_iterator_result`. Their runtime helpers already return an owned `Mixed` cell but do not set the compiled-return ownership marker.

On Linux x86_64 without heap debugging, their call to `__rt_incref` leaves `r11` at zero. The new check then treats the owned result as borrowed and retains it again, leaking a reference for each yielded key or value. Keep these already-owned runtime results out of the compiled-return check, or explicitly mark them owned.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Assembly comments miss required format

The new assembly in lower_dynamic_filter_attach has no // -- description -- group comments. The AArch64 branch on this line also puts // directly after the code without a space.

The contributor guide requires group comments and column-81 alignment, or exactly one separating space when the code is longer. Apply that format to the new instruction groups. This repository requirement must be satisfied before merging.

Context Used: AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/codegen/lower_inst/builtins/io/filter_dispatch.rs
Line: 45

Comment:
**Assembly comments miss required format**

The new assembly in `lower_dynamic_filter_attach` has no `// -- description --` group comments. The AArch64 branch on this line also puts `//` directly after the code without a space.

The contributor guide requires group comments and column-81 alignment, or exactly one separating space when the code is longer. Apply that format to the new instruction groups. This repository requirement must be satisfied before merging.

**Context Used:** AGENTS.md ([source](https://github.com/illegalstudio/elephc/blob/main/AGENTS.md))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Greptile has paused reviews on this repository — it used its 750 free open-source review credits for this billing period. Reviews resume automatically on November 12. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.

[nahime0]

@greptileai