splice: a negative start, and a left-out or negative delete count - #385
Merged
Merged
Conversation
`a.splice(-1, 1)` faulted: start went to index by zero extension (s32) or as-is, and the lowering took -1 as the largest unsigned index. `a.splice(2)` read an operand that was not there. JavaScript's rules now apply: start and delete count go to index through a signed 64-bit integer, and the lowering reads them signed - a negative start counts from the end and stops at 0, a start past the end is the end, a negative delete count deletes nothing, and a left-out delete count removes everything from start on (the existing clamp takes it down to what is there). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
The native array
splicehandled only the plain case. Found while addingspliceto DefaultLib'sArray<T>class (ASDAlexander77/TypeScriptCompilerDefaultLib#13, for #231):a.splice(-1, 1)a.splice(-10, 1)a.splice(2)a.splice(1, -3)a.splice(10, 1)Cause
index. From ans32literal that cast is a zero extension (indexis not a signed type), so-1arrived as 4294967295. The lowering then used the value as an unsigned offset.mlirGenArraySplice(operands)readoperands[2]whether or not it was there.Fix
MLIRCodeLogic.h:indexthrough a signed 64-bit integer, so the sign survives;INT32_MAX, which is positive in a 32-bit index as well; the lowering's existing clamp brings it down to what is there;ArraySpliceOpLowering: start and delete count are read as signed:This happens before the existing clamp and before the rc release of the removed elements, so both see the corrected values.
Tests
00array_splice.tsgains six cases: a negative start, a start before 0, a start past the end, a left-out delete count, a negative delete count, and anumberstart. It already runs in compile, jit,jit -mm=rcand the corpus. The old build faults on it.test-compile-none-corpus-00for-awaitfailed once in the full run and passed three times on rerun; that is the known flaky async test.Not in this PR
a.splice(1, 1, ...x)drops the spread items silently, anda.push(...x)fails verification. A spread argument never reaches the array builtins as its elements.splicereturns the new length, not the removed elements as in JavaScript. DefaultLib'sArray<T>.splice(DefaultLib#13) returns the removed elements.🤖 Generated with Claude Code