webidl icon indicating copy to clipboard operation
webidl copied to clipboard

Possible mistake in overload resolution algorithm

Open bathos opened this issue 3 years ago • 2 comments

Example Web IDL/JS first to help connect the overload step where I think there’s a mistake to something more concrete (attack crabs):

interface Crab {
  constructor();
  undefined attack(DOMString target);
  undefined attack((sequence<DOMString> or boolean) targets);
};
let crab = new Crab;
crab.attack("Misha"); // good
crab.attack(true); // convenient shorthand for attacking everyone also good
crab.attack([ "Misha", "Logan" ]); // this is busted I think? —

Screenshot of step 14 highlighting “Otherwise T is a FrozenArray type” with overlaid annotation saying T is actually a union at this step for this example

At step 12.9 we match the sequence-likes case because a type at the discriminating index (0) is “a union type, nullable union type, or annotated union type that has one of the above types in its flattened member types” and we get a callable result for GetMethod(V, @@iterator) (retained as method). At step 14, though, where method gets used, the T of “Let T be the type at index i in the type list of the remaining entry in S.” is (sequence<DOMString> or boolean) rather than sequence<DOMString>.

Since the intention is still clear, it seems like this is just an oversight in the spec text (unless I’m just misreading something else). I’m guessing implementations already do the obvious thing and select the member of T that’s a sequence-like.


The two 👀 reactions on this are from the real Misha and Logan, who may have bigger things to worry about than specification errors.

bathos avatar Aug 08 '21 17:08 bathos

The tricky part here is that we need to convert V to the eventual target type (whether that be a union type as here, or a nullable type, or an annotated type) without getting the @@iterator property again. We could potentially add an optional "iterator getter method" parameter to the algorithm "converting an ECMAScript value to an IDL value", and pass that value through the conversion algorithms for union types, nullable types, and annotated types.

TimothyGu avatar Aug 10 '21 21:08 TimothyGu

An abstraction over “iterator getting ... and using” might make sense, yeah, though it doesn’t seem like it would fit neatly into the three+ places it’s needed in slightly different ways (I mean, maybe. I could be picturing this wrong). I think though that this specific case can be repaired without that, for example by introducing a second var for retaining a reference to the specific type which met the criteria in step 12.9 or perhaps by asserting in 14 that T is or includes (as inner type, member type, ...) exactly one sequence-like type (this is assured) and that that is the type being “used” rather than T (if they are not one and the same).

bathos avatar Aug 12 '21 00:08 bathos