Skip to content

Interop: a params overload is chosen by the array's element type, and a failing element declines instead of throwing - #3764

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:3746
Sep 3, 2026
Merged

lahma merged 1 commit into
sebastienros:mainfrom
lahma:3746

Conversation

@lahma

@lahma lahma commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Answers discussion #3746, where three host overloads
differing only in the element type of a params array always selected the first-declared one and died
converting to it:

public CDecimal Add(params CDecimal[] args);
public CInteger Add(params CInteger[] args);
public CLong    Add(params CLong[] args);

CDecimal, CInteger and CLong are unrelated host classes and none of them is IConvertible, so
math.Add(a, b) with wrapped CInteger instances left Engine.Evaluate as
System.InvalidCastException: Object must implement IConvertible. — invisible to a script try/catch and
to a host catch (JavaScriptException) alike.

There are two defects on that path, and they are coupled: fixing the first by asking the converter per
element throws until the second is fixed. Hence one pull request.

1. A JS array's element type was invisible to overload scoring

MethodInfoFunction.ProcessParamsArrays bundles the trailing arguments into one JsArray before scoring, so
InteropHelper.CalculateMethodParameterScore rated a single argument against CDecimal[], CInteger[] and
CLong[] alike, through one rule:

if (parameterValue.IsSpecArray() && (paramType.IsArray || IsGenericCollectionType(paramType)))
{
    // we have potential, TODO if we'd know JS array's internal type we could have exact match
    return 2;
}

A flat 2 for every candidate. The element type was never consulted, so the converter probe below that rule —
the #3407 fix, which would have declined
CInteger -> CDecimal — was never reached, MethodMatch.CompareTo orders on Score alone, and declaration
order decided
. It also picked the wrong overload silently whenever the earlier candidate did happen to be
convertible: f(params string[]) declared before f(params int[]) answered the string concatenation "12"
for f(1, 2), because a double element scores 3 against string through CanChangeType while the array as
a whole scored a blanket 2 either way. Both were measured, not predicted — see the failing-first counts below.

Up to the first eight elements are now rated against the parameter's element type, and the parameter scores
ArrayParameterBaseScore (still 2) plus its worst element, never their sum, so an exact-typed array
answers exactly what it answered before and a long array cannot outweigh the parameters beside it. The branch
never returns 0 — a perfect score ends FindBestMatch outright, and an array argument has never been a
perfect match. An element no conversion can produce short-circuits the candidate to -1, which is the same
rule #3407 applies one level up: the last word belongs to the converter that will actually perform the
conversion.

Two element types are carved out and keep the flat base score: object and JsValue — anything a JsValue
is already assignable to. params object[] and params JsValue[] are what a host writes for "anything", and
rating their elements would rank them below whatever scalar overload sits beside them.

2. The composite branches of TryConvert threw instead of declining

DefaultTypeConverter converted the parts of a composite through the public, throwing Convert at five
sites — a List<T> item, a Collection<T> item, a T[] element, a target dictionary's value, a member of a
POCO built from a dictionary — ignoring the propagateException flag their own frame had been called with. So
TryConvert, documented as "returning false if the conversion cannot be done", threw. That is what defeated
the recovery already built into MethodInfoFunction.Call, which tries candidates in score order and is meant
to move on when one declines: the only catch in TryCall is for TargetInvocationException around the
invoke, not around the conversion.

A new private TryConvertPart routes all five through the flag their frame was given. Its body mirrors
Convert — the virtual TryConvert first, so a subclass override still answers for the parts, then, only
when propagating, TryConvertInternal(..., propagateException: true, ...) and Throw.Error on failure — so
behaviour under propagateException: true is unchanged and Convert throws exactly the same exception for
exactly the same input as before.

Independently of overload resolution, this is what an embedder converting a JS array, a List<T>, a
dictionary or a POCO with one unconvertible part got out of a Try method.

Failing-first

Both new files were written and run against unfixed code first.

Suite Legs Before After
Jint.Tests.PublicInterface/HostArrayElementOverloadTests.cs net472, net8.0, net10.0 8 failed / 3 passed per leg (24 failures) 11/11 per leg
Jint.Tests/Runtime/Interop/CompositeConversionDeclineTests.cs net472, net8.0, net10.0 5 failed / 6 passed per leg (15 failures) 11/11 per leg

The pre-fix messages were the report's own: System.InvalidCastException : Object must implement IConvertible
out of Evaluate for the three-overload case and for the explicit-array and List<T> cases;
Expected a <Jint.Runtime.JavaScriptException> to be thrown, but found <System.InvalidCastException> for the
unbindable-argument cases on both the method and the constructor lane; "12" where "3" was expected for the
silent wrong answer; and Did not expect any exception, but found System.InvalidCastException for each of the
five converter sites. The three that passed before and after are the deliberate no-change pins: the empty
array, and the two carve-out guards.

Deliberately out of scope

  • MethodDescriptor.Call (the TypeReference constructor lane) converts with the throwing Convert and
    has no candidate loop at all. Fixing defect 1 is what repairs it — correct selection means the argument it
    is handed is bindable — exactly as Interop: a plain type mismatch scores 100 instead of being discarded, so 'text' + an overloaded CLR value throws #3407 repaired that same lane. No retry loop was added there.
  • An empty JS array against several params T[] overloads stays genuinely ambiguous and keeps today's
    declaration-order answer. C# reports an ambiguity error there; the engine has no better answer available.
    Pinned so a future element rule cannot change it quietly.
  • When every candidate declines, the CLR exception's detail is no longer in the message.
    Options.Interop.ExposeDetailedResolutionErrors already names the argument and candidate types; surfacing
    the underlying exception too is a follow-up at most.

Also in this change

Verification

Rebased onto main at 4939c85, then a full dotnet build -c Release and dotnet test -c Release: every
suite green — Jint.Tests 12239/12239 (net8.0, net10.0) and 8468/8468 (net472),
Jint.Tests.PublicInterface 3601/3591/2874, Jint.Tests.Browser, Jint.Tests.DevTools,
Jint.Tests.CommonScripts, Jint.Tests.SourceGenerators. Test262 102569/102688, the four failures being
intl402/supportedLocalesOf-unicode-extensions-ignored.js and staging/sm/Array/toSpliced-dense.js ×2 each,
which are the known 30-second load flakes and pass in isolation (186/186). The host-contract leg
(JINT_HOST_CONTRACT_VERIFICATION=1 over Jint.Tests.PublicInterface) is green on all three legs.

No benchmark is owed: scoring runs only for overloaded members, and the branch already materialises the whole
array through ToObject() before reaching it.

Closes #3754

🤖 Generated with Claude Code

https://claude.ai/code/session_01LfyCDDqtkhyWVEFHKWhZfu

… a failing element declines instead of throwing

A `params` call bundles its trailing arguments into one `JsArray` before overload
resolution runs, so `InteropHelper.CalculateMethodParameterScore` rated three
candidates differing only in their element type - `CDecimal[]`, `CInteger[]`,
`CLong[]` - with one rule, "is an array, wants an array", that answered a flat 2
for every one of them. The element type was never consulted, the converter probe
below that rule (the sebastienros#3407 fix) was never reached, and declaration order decided.
Where the first-declared candidate happened to be convertible it answered
silently and wrongly; where it was not, the conversion's `InvalidCastException`
left through the embedder's `Evaluate`.

It escaped because of the second defect. The composite branches of
`DefaultTypeConverter.TryConvert` - a `List<T>` or `Collection<T>` item, a `T[]`
element, a target dictionary's value, a member of a POCO built from a dictionary
- converted their parts through the public, throwing `Convert`, ignoring the
`propagateException` flag their own frame had been called with. So a method
documented as returning false threw, which is not something the candidate loop in
`MethodInfoFunction.Call` can move on from: it tries candidates in score order and
declines its way to the next, and an exception ends the call rather than the
candidate.

Both are fixed together, because fixing the first by asking the converter per
element throws until the second is fixed.

- `TryConvertPart` routes all five composite sites through the flag their frame
  was given. It mirrors `Convert` - the virtual `TryConvert` first, so a subclass
  override still answers for the parts - so behaviour under
  `propagateException: true` is unchanged.
- `CalculateParameterTypeScore` is the type-level core of the parameter score, so
  an element with no `ParameterInfo` of its own can be scored by the same rules.
  An array parameter now scores its base 2 plus its *worst* element, up to eight
  of them, and returns -1 on the first element that cannot bind at all. Element
  types any `JsValue` is assignable to - `object`, `JsValue` - are carved out and
  keep the flat base, or `params object[]` and `params JsValue[]` would rank
  below the scalar overload beside them.

Deliberately out of scope: `MethodDescriptor.Call`'s `TypeReference` constructor
lane keeps its throwing `Convert` and gets no retry loop - correct selection is
what repairs it, exactly as sebastienros#3407 repaired that lane - and an empty JS array
against several `params T[]` overloads stays ambiguous and keeps today's answer.

Pinned in `Jint.Tests.PublicInterface/HostArrayElementOverloadTests.cs` and
`Jint.Tests/Runtime/Interop/CompositeConversionDeclineTests.cs`.

Closes sebastienros#3754

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LfyCDDqtkhyWVEFHKWhZfu
@lahma
lahma merged commit 0fb7c0c into sebastienros:main Sep 3, 2026
8 checks passed
lahma added a commit that referenced this pull request Sep 3, 2026
… a failing element declines instead of throwing (backport of #3764) (#3782)

Backport of #3764, whose description carries the full mechanism. Both defects are present on 4.x:
overload scoring short-circuits a JavaScript array against any array or generic-collection parameter
to a flat score, so `params` overloads differing only in element type tie and declaration order
decides; and `DefaultTypeConverter` converts the parts of a composite through the public throwing
`Convert`, so `TryConvert` throws where it documents that it returns false and the candidate-decline
loop in `MethodInfoFunction.Call` never reaches the overload that would have bound.

Four divergences from main, none of them behavioural. The spec-array predicate is spelled `IsArray()`
here. The POCO-member copy is inline in a `GetMembers()` loop rather than in the static local function
main extracted, so `TryConvertPart` is called in place and main's refactor was not imported. The
constructor-resolution message is worded differently, so that assertion matches 4.x's wording. And
there is no `Jint/Runtime/Interop/AGENTS.md` on this branch and no `docs/v5-migration.md`, so neither
gains a row.

4.x carries the #3407 converter probe the element lane depends on for its refusals. It does not carry
the #3577 numeric-range gate, which this change does not need.

Both new test files fail first on unfixed 4.x, eight of eleven and five of eleven, identically on
net472 and net10.0. test262 lands exactly on the branch control, 102,499 passed and 0 failed of
102,684, with neither known load flake firing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A JS array's element type is invisible to overload scoring, and a failing element makes TryConvert throw instead of decline

1 participant