Stop sort throwing a CLR ArgumentException at script on net8 and net10 - #2954
Merged
lahma merged 1 commit intoAug 10, 2026
Merged
Conversation
Polyfills.Order backfilled a merge sort for the frameworks that lack Enumerable.Order, and the claim that came with it was that Array.prototype.sort, toSorted and %TypedArray%.prototype.sort behave identically on all five target frameworks. They do not. The guard was #if !NET8_0_OR_GREATER, so net8.0 and net10.0 still bind Enumerable.Order and still sort with the BCL introsort. That introsort does not spin the way .NET Framework's quicksort does, but it does not finish either: it detects the inconsistency and throws ArgumentException. An inconsistent comparison function is legal JavaScript -- https://tc39.es/ecma262/#sec-sortcompare leaves the resulting order implementation-defined and https://tc39.es/ecma262/#sec-sortindexedproperties still requires the sort to complete. And ArgumentException is not a JavaScriptException, while the three call sites unwrap only InvalidOperationException, so `try { a.sort(function () { return -1; }); } catch (e) {}` never caught it and the exception escaped engine.Evaluate entirely. The merge sort therefore leaves Polyfills, where an unconditional helper does not belong, and becomes SortExtensions.StableOrder. Being the one implementation on every target framework is also what finally makes the identical-order claim true. It hands back the array it sorted instead of an IEnumerable, so the two call sites that wanted an array stop copying it a second time. The regression test passed vacuously above net462: 20 elements is over .NET Core's 16-element insertion-sort threshold, but `a === 1 ? -1 : 1` happens never to trip the detector. It now also runs the blunter `return -1`, which failed all three sorts on net10.0 before this change and none after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lahma
added a commit
to lahma/jint
that referenced
this pull request
Aug 10, 2026
…Date new Date(253402300800000).toISOString() threw ArgumentOutOfRangeException, "The added or subtracted value results in an un-representable DateTime", out of engine.Evaluate. Not a JavaScriptException, so a script try/catch around the call never saw it -- the same escape sebastienros#2954 closed for sort. toJSON, JSON.stringify and toString went the same way at that value, toString through DateTimeOffset.FromUnixTimeMilliseconds instead, whose message helpfully names 253402300799999 as the largest value it accepts. That value is the first millisecond of year 10000. It is well inside the legal Date range -- https://tc39.es/ecma262/#sec-timeclip admits every time value up to 8.64e15 -- and one millisecond past what DateTime can represent, which is the whole of the failing band: 253402300799999 works, 253402300800001 works, and nothing else is in between. The arithmetic slow path was never the problem; it is long math and renders year 10000 fine. What is wrong is the bound that decides whether a value can skip it. JsDate.Max read DateTime.MaxValue's distance from the epoch off TimeSpan.TotalMilliseconds, which is a double division: the true quotient is 253402300799999.9999, doubles are spaced 2^-5 apart up there, and the correctly rounded result is 253402300800000.0 exactly. So the bound landed one millisecond above the last representable instant and DateTimeRangeValid answered true for a value DateTime cannot hold, sending it into a conversion that could only throw. Counting ticks instead is exact, and the value now takes the slow path like every other year outside DateTime's reach. JsDate.Min is counted the same way for symmetry but does not move: the epoch is a whole number of milliseconds after DateTime.MinValue, so that division was already exact. One consequence is worth stating. DatePresentation.MaxValue carries this bound, so a host passing DateTime.MaxValue to the JsDate constructor used to get a Date sitting in year 10000 -- one whose toISOString threw. It now gets 9999-12-31T23:59:59.999Z, and ToDateTime still returns DateTime.MaxValue exactly, which is what the DateTimeMaxValue flag has always been for. Left alone: toLocaleString converts unconditionally and so throws the same CLR exception for every date outside DateTime's range, at both ends. That is a wider defect than this bound and wants its own fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lahma
added a commit
that referenced
this pull request
Aug 10, 2026
…Date (#2965) new Date(253402300800000).toISOString() threw ArgumentOutOfRangeException, "The added or subtracted value results in an un-representable DateTime", out of engine.Evaluate. Not a JavaScriptException, so a script try/catch around the call never saw it -- the same escape #2954 closed for sort. toJSON, JSON.stringify and toString went the same way at that value, toString through DateTimeOffset.FromUnixTimeMilliseconds instead, whose message helpfully names 253402300799999 as the largest value it accepts. That value is the first millisecond of year 10000. It is well inside the legal Date range -- https://tc39.es/ecma262/#sec-timeclip admits every time value up to 8.64e15 -- and one millisecond past what DateTime can represent, which is the whole of the failing band: 253402300799999 works, 253402300800001 works, and nothing else is in between. The arithmetic slow path was never the problem; it is long math and renders year 10000 fine. What is wrong is the bound that decides whether a value can skip it. JsDate.Max read DateTime.MaxValue's distance from the epoch off TimeSpan.TotalMilliseconds, which is a double division: the true quotient is 253402300799999.9999, doubles are spaced 2^-5 apart up there, and the correctly rounded result is 253402300800000.0 exactly. So the bound landed one millisecond above the last representable instant and DateTimeRangeValid answered true for a value DateTime cannot hold, sending it into a conversion that could only throw. Counting ticks instead is exact, and the value now takes the slow path like every other year outside DateTime's reach. JsDate.Min is counted the same way for symmetry but does not move: the epoch is a whole number of milliseconds after DateTime.MinValue, so that division was already exact. One consequence is worth stating. DatePresentation.MaxValue carries this bound, so a host passing DateTime.MaxValue to the JsDate constructor used to get a Date sitting in year 10000 -- one whose toISOString threw. It now gets 9999-12-31T23:59:59.999Z, and ToDateTime still returns DateTime.MaxValue exactly, which is what the DateTimeMaxValue flag has always been for. Left alone: toLocaleString converts unconditionally and so throws the same CLR exception for every date outside DateTime's range, at both ends. That is a wider defect than this bound and wants its own fix. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
lahma
added a commit
to lahma/jint
that referenced
this pull request
Aug 11, 2026
…ocaleString new Date(8640000000000000).toLocaleString() threw ArgumentOutOfRangeException, "Value to add was out of range", out of engine.Evaluate. Not a JavaScriptException, so a script try/catch around the call never saw it -- the same escape sebastienros#2954 closed for sort and sebastienros#2965 for the first instant of year 10000, whose message named this one as left alone. toLocaleDateString and toLocaleTimeString went the same way, and so did both ends of the range: 253402300800001 and 8640000000000000 above, -62135596800001 and -8640000000000000 below. All three converted the time value with DatePresentation.ToDateTime() before handing it to the ECMA-402 path, and that conversion is unguarded. DateTime spans years 1 through 9999. https://tc39.es/ecma262/#sec-timeclip admits every time value up to 8.64e15, which is years -271821 through 275760, so a perfectly legal Date can name a year no calendar on the platform can hold. Those years now get the culture-independent rendering their non-locale siblings already give the same value: toLocaleString answers what toString does, toLocaleDateString what toDateString does, toLocaleTimeString what toTimeString does. https://tc39.es/ecma262/#sec-datestring and #sec-timestring are integer arithmetic on the time value and render year 275760 as readily as year 2026, which is why toString never had the problem. Two better-looking alternatives were tried and rejected. Clamping the DateTime to MinValue/MaxValue and overriding only the year is what Intl.DateTimeFormat.prototype.format does today, and it is worse than it sounds: month, day and time come from the clamp, so new Date(8640000000000000) formats as "12/31, 27576011:59:59 PM" -- December 31 rather than September 13, the time from DateTime.MaxValue, and the separators misplaced because BuildFormatString reads the year literal that lane emits as an hour literal. Carrying the real fields in on a substitute year congruent mod 400 would render what node renders, but it needs the whole component pipeline to learn about per-field overrides it does not have, for a band of years no non-Gregorian calendar can express anyway. Being wrong in the locale's shape is worse than being right in nobody's. The formatter is still constructed before the switch, so CreateDateTimeFormat still reads the locale list and the options bag and still rejects what the spec says to reject: new Date(8640000000000000).toLocaleString('!!bad!!') is a RangeError, as it was. One more CLR exception on the same path, at a value inside DateTime's range. An explicit numeric offset shifts the wall clock past the end of DateTime -- toLocaleString('en-US', { timeZone: '+03:00' }) at 253402300799999 -- and the offset branch of ConvertToTimeZone applied it with DateTime.Add, which throws. It saturates now, which is what TimeZoneInfo.ConvertTimeFromUtc, the named-zone branch beside it, has always done. Left alone: Intl.DateTimeFormat.prototype.format still mis-renders these years through the clamp-plus-originalYear lane described above. That is a wrong string rather than an exception escaping the engine, and it wants its own fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lahma
added a commit
that referenced
this pull request
Aug 11, 2026
new Date(8640000000000000).toLocaleString() threw ArgumentOutOfRangeException, "Value to add was out of range", out of engine.Evaluate. Not a JavaScriptException, so a script try/catch around the call never saw it -- the same escape #2954 closed for sort and #2965 for the first instant of year 10000, whose message named this one as left alone. toLocaleDateString and toLocaleTimeString went the same way, and so did both ends of the range: 253402300800001 and 8640000000000000 above, -62135596800001 and -8640000000000000 below. All three converted the time value with DatePresentation.ToDateTime() before handing it to the ECMA-402 path, and that conversion is unguarded. DateTime spans years 1 through 9999. https://tc39.es/ecma262/#sec-timeclip admits every time value up to 8.64e15, which is years -271821 through 275760, so a perfectly legal Date can name a year no calendar on the platform can hold. Those years now get the culture-independent rendering their non-locale siblings already give the same value: toLocaleString answers what toString does, toLocaleDateString what toDateString does, toLocaleTimeString what toTimeString does. https://tc39.es/ecma262/#sec-datestring and #sec-timestring are integer arithmetic on the time value and render year 275760 as readily as year 2026, which is why toString never had the problem. Two better-looking alternatives were tried and rejected. Clamping the DateTime to MinValue/MaxValue and overriding only the year is what Intl.DateTimeFormat.prototype.format does today, and it is worse than it sounds: month, day and time come from the clamp, so new Date(8640000000000000) formats as "12/31, 27576011:59:59 PM" -- December 31 rather than September 13, the time from DateTime.MaxValue, and the separators misplaced because BuildFormatString reads the year literal that lane emits as an hour literal. Carrying the real fields in on a substitute year congruent mod 400 would render what node renders, but it needs the whole component pipeline to learn about per-field overrides it does not have, for a band of years no non-Gregorian calendar can express anyway. Being wrong in the locale's shape is worse than being right in nobody's. The formatter is still constructed before the switch, so CreateDateTimeFormat still reads the locale list and the options bag and still rejects what the spec says to reject: new Date(8640000000000000).toLocaleString('!!bad!!') is a RangeError, as it was. One more CLR exception on the same path, at a value inside DateTime's range. An explicit numeric offset shifts the wall clock past the end of DateTime -- toLocaleString('en-US', { timeZone: '+03:00' }) at 253402300799999 -- and the offset branch of ConvertToTimeZone applied it with DateTime.Add, which throws. It saturates now, which is what TimeZoneInfo.ConvertTimeFromUtc, the named-zone branch beside it, has always done. Left alone: Intl.DateTimeFormat.prototype.format still mis-renders these years through the clamp-plus-originalYear lane described above. That is a wrong string rather than an exception escaping the engine, and it wants its own fix. Co-authored-by: Claude Fable 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
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.
The merge sort #2898/#2899 added is guarded
#if !NET8_0_OR_GREATER, so on net8.0 and net10.0 the three script-visible sorts still bindEnumerable.Order, whose introsort raisesArgumentException("Unable to sort because the IComparer.Compare() method returns inconsistent results…"). That is a CLR exception, not aJavaScriptException, so scripttry/catchcannot see it and it escapesengine.Evaluate— the three call sites unwrapInvalidOperationException, andArgumentExceptionis not one.A comparison function that never returns 0 and is not antisymmetric is legal JavaScript: sec-sortcompare leaves the order implementation-defined, but the sort must finish.
Two things made this survive review. The throw is pre-existing on net8/net10 — v4.15.3 used
OrderforsortandOrderByfor the other two, sameEnumerableSorter— so what this release added was the claim, in #2899's message, that all three "behave identically on all five target frameworks". And the regression test guarding it passed vacuously:ArrayTests.SortTerminatesWithAnInconsistentComparatoruseda === 1 ? -1 : 1over 20 elements, which happens not to trip the detector. It is now a 6-row theory (3 sorts × 2 comparators) includingreturn -1, and 3 rows are red onmainon net10.0 while net472 was already green — the split this PR closes.The merge sort moves out of
Polyfills.cs— it is not a polyfill any more — intoJint/Extensions/SortExtensions.csasStableOrder, unconditional on all five target frameworks. The algorithm is unchanged; it returnsT[]rather thanIEnumerable<T>, sotoSortedand%TypedArray%.sortdrop their.ToArray(). That also delivers what #2899's message promised: one identical result order everywhere.What changes on net8/net10, measured by counting rather than timing:
sort/toSorted/Int32Array.sortsortsortsortFewer JS comparator calls on random and already-sorted data, more on reverse-sorted. Allocation shifts too: one full-length auxiliary buffer replaces the
int[]index map, sosortis about +4n bytes andtoSorted/%TypedArray%.sortabout −4n bytes with one fewer array. net462/netstandard execute exactly what they did before.Only two benchmark rows reach the change:
ArraySortBenchmark.SortWithComparer_1K(the other five call.sort()with no comparator and go throughSortByCachedStringKeys, so they are controls), andArrayCopyingMethodsBenchmark.ToSorted_Dense, where the native comparer makes each comparison cheap and the merge sort's own bookkeeping is most exposed.Test262 99,738 / 0 / 157 — no ordering change surfaces there.
🤖 Generated with Claude Code