refactor(core): apply bug fixes on methods#341
Open
Guikingone wants to merge 8 commits into
Open
Conversation
A string builtin (strtoupper/md5/str_repeat/...) or a runtime concatenation materializes its result into the shared _concat_buf scratch and returns a borrowed slice. Passing such a slice as a function/method/closure argument produced garbage: the callee reset _concat_off to 0 at its first statement and its own concats overwrote the caller's slice bytes before the callee read them. Each frame now captures, on entry, the _concat_off it inherits from the caller (the high-water mark below which the caller's live slices sit) into a hidden slot, and the per-statement reset restores _concat_off to that base instead of 0. The callee's concats append above the caller's slices instead of clobbering them. main and other root contexts keep base 0 (unchanged). The existing save/restore of the cursor around nested calls composes with this. No heap allocation and no extra frees: pure cursor arithmetic, so there is no ownership or dangling-pointer hazard. Adds tests/codegen/regressions/concat_buffer_args.rs (function/method/closure/ chained pass-through/md5/two-arg) and updates docs/internals/memory-model.md.
__rt_ftruncate (x86_64) read the truncation size from rdx, but the ftruncate builtin emitter places it in rsi — the same SysV second-arg position the synthetic user-wrapper dispatch path uses. The stray `mov rsi, rdx` clobbered the correct size with garbage, so ftruncate() resized real-fd files to a bogus length on x86_64. This failed ftruncate()/SplFileObject::ftruncate()/ SplTempFileObject-spill tests on linux-x86_64 only; arm64 was unaffected because both the builtin and runtime use x1 for the size there. The size already arrives in rsi from the caller, so only the fd needs moving into rdi. The user-wrapper path (already rsi-based) is unchanged. Fixes the 4 x86_64-only failures; the existing ftruncate/SPL tests cover the regression.
abs() checked only `ty == Float` and otherwise assumed a raw integer in the result register. For a boxed Mixed/Union operand that register holds a pointer to the boxed cell, so abs() computed the absolute value of the pointer and printed garbage (e.g. `echo abs($heterogeneousArrayElement)`). Add a `__rt_abs_mixed` runtime helper (both arches, modeled on mixed_numeric_binops): unbox the payload, branch on the runtime tag — float clears the IEEE-754 sign bit and stays float, integer uses a branchless absolute value, other tags coerce through __rt_mixed_cast_int — then rebox via __rt_mixed_from_value so PHP's int->int / float->float typing is preserved. abs.rs dispatches Mixed/Union operands to it and returns Mixed; infer_local_type's abs arm infers Mixed for a Mixed/Union argument so the local slot type matches the emitter. Plain int/float abs is unchanged. Adds a regression test over a heterogeneous array. (Assigning abs() of a heterogeneous-array float element into a typed local can still lose the float type; that is a separate upstream element-typing issue, not abs.)
pow() converted a non-float operand to double with scvtf/cvtsi2sd, which for a boxed Mixed/Union operand converted the boxed-cell pointer instead of the value, producing garbage (e.g. pow($mixed, 2)). Since elephc's pow() already returns a double via libc pow(), the fix only needs the numeric value: route Mixed/Union base and exponent operands through __rt_mixed_cast_float (boxed -> double) before the call, on both arches. Plain int/float operands are unchanged. Adds a regression test covering a Mixed base, Mixed exponent, and both Mixed.
is_float was the lone type predicate that constant-folded to false for a boxed Mixed/Union argument instead of inspecting the runtime tag, unlike its siblings (is_int/is_string/is_bool/is_null). It now unboxes via __rt_mixed_unbox and tests tag 2 (float), matching PHP for heterogeneous array elements and mixed-typed values.
…on x86_64 Two related float-predicate bugs. 1. is_nan/is_finite/is_infinite normalized a non-float argument with emit_int_result_to_float_result, which for a boxed Mixed/Union operand converted the cell pointer rather than the value (e.g. iterating a heterogeneous float/int array). They now route Mixed/Union through __rt_mixed_cast_float before the IEEE check, matching the sibling predicates. 2. is_infinite(NAN) returned true on x86_64: the ucomisd/sete infinity check sets the zero flag for the unordered NaN comparison, so NaN looked equal to +/-Inf. Added a NaN parity guard (NaN is unordered, never infinite), mirroring is_finite. arm64 was already correct via fcmp/cset. Adds regression tests for the Mixed array case and for is_infinite(NAN).
is_numeric() dispatched only on the compile-time type, so a boxed Mixed/Union operand (e.g. a heterogeneous-array element) always returned false even for an int, float, or numeric-string value. Extract the inline numeric-string scan into a shared emit_numeric_string_scan helper and add a Mixed/Union arm that unboxes the runtime tag: int and float are numeric, a string payload is run through the same scan, every other tag is not numeric. The non-Mixed paths are unchanged. Adds a regression test over a heterogeneous int/float/numeric-string/non-numeric-string/bool array.
array_fill ignored a non-zero start index (array_fill(5, 2, 7) produced keys
0,1 instead of 5,6) and aborted on a string fill value (it was routed to the
8-byte scalar runtime, which dropped the string length). A 0-based indexed
array cannot represent keys like 5,6,7, so those cases need a keyed (hash)
array.
Add __rt_array_fill_assoc (both arches): build a Mixed-valued hash with keys
start..start+count-1, boxing each slot via __rt_mixed_from_value so per-slot
string/refcounted copy/share semantics stay correct. The builtin routes a
non-literal-zero start, or a string value, to it and returns
AssocArray{Int, Mixed}; a literal-zero start with a scalar/refcounted value
keeps the unchanged indexed path. infer_local_type and the type checker are
updated to the same dispatch so the result type stays consistent.
Verified against PHP for int/float/string/array values and negative starts; the
fill loop is GC-clean and existing start-0 tests are unaffected. Adds regression
tests for the keyed path and the unchanged indexed path.
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
1 participant
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.
No description provided.