Skip to content

Commit

Permalink
Fully implement overloaded and variadic ops
Browse files Browse the repository at this point in the history 
Fixes jsdom#5.
Fixes jsdom#29.
  • Loading branch information
@TimothyGu
TimothyGu committed Oct 15, 2017
1 parent b46ddba commit 21d86d0
Show file tree
Hide file tree
Showing 8 changed files with 1,383 additions and 840 deletions.
46 changes: 4 additions & 42 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,49 +228,11 @@ However, note that apart from Web IDL container return values, this impl-back-to

The same holds true in reverse: if you accept some other container as an argument, then webidl2js will not automatically be able to find all the wrappers it contains an convert them into impls; you will need to use `implFromWrapper` before processing them.

#### Overloaded operations

One other subtlety here is overloads: if the IDL file defines overloads for a given operation, webidl2js is not always as helpful as it could be.

1. webidl2js does not yet implement the [overload resolution algorithm](https://heycam.github.io/webidl/#dfn-overload-resolution-algorithm) fully. Take the `overload1()` operations below:

```webidl
void overload1(DOMString first, long second);
void overload1(DOMString first, long second, DOMString third);
```

webidl2js fully handles type conversion for both `first` and `second` arguments, but does not try to convert the third argument even if it is provided.

Similarly, consider the following `overload2()` operations:

```webidl
void overload2(Blob first, long second);
void overload2(long first, long second);
```

webidl2js only converts `second` argument because its type is unambiguous, and calls the implementation method with `first` unconverted. In particular, this means that webidl2js will not try to unwrap the `first` argument into a `Blob` implementation class, even if it is a `Blob`.

2. webidl2js does not dispatch overloaded operations to separate implementation class methods, but instead performs type conversions if possible and then sends the result to the same backing method.
Variadic operations are fully supported by webidl2js.

We're hoping to fix both of these problems in [#29](https://github.com/jsdom/webidl2js/issues/29). But in the meantime, properly implementing overloads requires doing some extra type-checking (often using appropriate [`is()`](#isvalue) functions) to determine which case of the overload you ended up in, and manual conversion with [webidl-conversions][] and/or unwrapping.

#### Variadic operations

Variadic operations are fully supported by webidl2js, but only if the particular operation is not overloaded. So while type conversions for the `simple1` and `simple2` operations below are fully implemented, webidl2js will not provide any variadic semantics for `overloaded1()` or `overloaded2()`.

```webidl
void simple1(DOMString... strings);
void simple2(DOMString first, URL... urls);
void overloaded1(DOMString... strings);
void overloaded1(unsigned long... numbers);
void overloaded2(DOMString first, DOMString... strings);
void overloaded2(unsigned long first, DOMString... strings);
```
#### Overloaded operations

We hope to fix this problem in the overload resolution overhaul ([#29](https://github.com/jsdom/webidl2js/issues/29)). Right now, however, manual conversions will be needed for overloaded variadic operations.
In the case of overloaded operations, webidl2js is not always as helpful as it could be. Due to the fact that JavaScript does not have a C++-like method overloading system, all overloads of a method are dispatched to the same implementation method, which means that the implementation class would need to do some secondary overload resolution to determine which overload is actually called.

#### Static operations

Expand Down Expand Up @@ -375,7 +337,7 @@ webidl2js is implementing an ever-growing subset of the Web IDL specification. S
- Basic types (via [webidl-conversions][])
- Mixins, i.e. `implements`
- Overload resolution (although [tricky cases are not easy on the implementation class](#overloaded-operations))
- Variadic arguments ([only non-overloaded operations](#variadic-operations) are supported though)
- Variadic arguments
- `[Clamp]`
- `[Constructor]`
- `[EnforceRange]`
Expand Down
20 changes: 5 additions & 15 deletions lib/constructs/interface.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,34 +314,24 @@ class Interface {

if (overloads.length !== 0) {
let minConstructor = overloads[0];

for (let i = 1; i < overloads.length; ++i) {
if (overloads[i].nameList.length < minConstructor.nameList.length) {
minConstructor = overloads[i];
}
}

const conversions =
Parameters.generateOverloadConversions(this.ctx, overloads, this.name, `Failed to construct '${this.name}': `);
const conversions = Parameters.generateOverloadConversions(
this.ctx, "constructor", this.name, this, `Failed to construct '${this.name}': `);
this.requires.merge(conversions.requires);

minConstructor.nameList = minConstructor.nameList.map(name => (keywords.has(name) ? "_" : "") + name);
const argNames = minConstructor.nameList.map(name => (keywords.has(name) ? "_" : "") + name);
this.str += `
function ${this.name}(${minConstructor.nameList.join(", ")}) {
`;
if (minConstructor.nameList.length !== 0) {
const plural = minConstructor.nameList.length > 1 ? "s" : "";
this.str += `
function ${this.name}(${argNames.join(", ")}) {
if (new.target === undefined) {
throw new TypeError("Failed to construct '${this.name}'. Please use the 'new' operator; this constructor " +
"cannot be called as a function.");
}
if (arguments.length < ${minConstructor.nameList.length}) {
throw new TypeError("Failed to construct '${this.name}': ${minConstructor.nameList.length} " +
"argument${plural} required, but only " + arguments.length + " present.");
}
`;
}
`;
this.str += conversions.body + "\n";

const passArgs = conversions.hasArgs ? ", args" : "";
Expand Down
38 changes: 20 additions & 18 deletions lib/constructs/operation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,22 @@ class Operation {
return firstOverloadOnInstance;
}

fixUpArgsExtAttrs() {
for (const idl of this.idls) {
for (const arg of idl.arguments) {
if (arg.extAttrs) {
if (!arg.idlType.extAttrs) {
arg.idlType.extAttrs = [];
}
arg.idlType.extAttrs.push(...arg.extAttrs);
}
}
}
}

generate() {
const requires = new utils.RequiresMap(this.ctx);
this.fixUpArgsExtAttrs();
let str = "";

if (!this.name) {
Expand All @@ -41,19 +55,18 @@ class Operation {

const type = this.static ? "static operation" : "regular operation";
const overloads = Overloads.getEffectiveOverloads(type, this.name, 0, this.interface);
let minConstructor = overloads[0];

let minOp = overloads[0];
for (let i = 1; i < overloads.length; ++i) {
if (overloads[i].nameList.length < minConstructor.nameList.length) {
minConstructor = overloads[i];
if (overloads[i].nameList.length < minOp.nameList.length) {
minOp = overloads[i];
}
}

const fnName = (keywords.has(this.name) ? "_" : "") + this.name;
minConstructor.nameList = minConstructor.nameList.map(n => (keywords.has(n) ? "_" : "") + n);
const argNames = minOp.nameList.map(name => (keywords.has(name) ? "_" : "") + name);

str += `
${targetObj}.${this.name} = function ${fnName}(${minConstructor.nameList.join(", ")}) {
${targetObj}.${this.name} = function ${fnName}(${argNames.join(", ")}) {
`;
if (!this.static) {
str += `
Expand All @@ -63,24 +76,13 @@ class Operation {
`;
}

if (minConstructor.nameList.length !== 0) {
const plural = minConstructor.nameList.length > 1 ? "s" : "";
str += `
if (arguments.length < ${minConstructor.nameList.length}) {
throw new TypeError("Failed to execute '${this.name}' on '${this.interface.name}': " +
"${minConstructor.nameList.length} argument${plural} required, but only " +
arguments.length + " present.");
}
`;
}

const callOn = this.static ? "Impl.implementation" : "this[impl]";
// In case of stringifiers, use the named implementation function rather than hardcoded "toString".
// All overloads will have the same name, so pick the first one.
const implFunc = this.idls[0].name || this.name;

const parameterConversions = Parameters.generateOverloadConversions(
this.ctx, overloads, this.interface.name, `Failed to execute '${this.name}' on '${this.interface.name}': `);
this.ctx, type, this.name, this.interface, `Failed to execute '${this.name}' on '${this.interface.name}': `);
const argsSpread = parameterConversions.hasArgs ? "...args" : "";
requires.merge(parameterConversions.requires);
str += parameterConversions.body;
Expand Down
76 changes: 33 additions & 43 deletions lib/overloads.js
View file
Original file line number Diff line number Diff line change
@@ -1,28 +1,30 @@
"use strict";

module.exports.getEffectiveOverloads = function (type, A, N, I) {
const S = [];
let F = null;
const { areDistinguishable, sameType } = require("./types");

function getOperations(type, A, I) {
switch (type) {
case "regular operation":
F = I.operations.get(A).idls;
break;
return I.operations.get(A).idls;
case "static operation":
F = I.staticOperations.get(A).idls;
break;
case "constructor":
F = I.idl.extAttrs.filter(a => a.name === "Constructor");
for (const c of F) {
return I.staticOperations.get(A).idls;
case "constructor": {
const ctor = I.idl.extAttrs.filter(a => a.name === "Constructor");
for (const c of ctor) {
if (!c.arguments) {
c.arguments = [];
}
}
break;
default:
throw new RangeError(`${type}s are not yet supported`);
return ctor;
}
}
throw new RangeError(`${type}s are not yet supported`);
}
module.exports.getOperations = getOperations;

module.exports.getEffectiveOverloads = function (type, A, N, I) {
const S = [];
const F = getOperations(type, A, I);
let maxArgs = 0;
for (const X of F) {
if (X.arguments.length > maxArgs) {
Expand All @@ -34,7 +36,7 @@ module.exports.getEffectiveOverloads = function (type, A, N, I) {
for (const X of F) {
const n = X.arguments.length;
const nameList = X.arguments.map(arg => arg.name);
const typeList = X.arguments.map(arg => arg.idlType.idlType);
const typeList = X.arguments.map(arg => arg.idlType);
const optionalityList = X.arguments.map(arg => {
if (arg.optional) {
return "optional";
Expand Down Expand Up @@ -87,40 +89,28 @@ module.exports.getEffectiveOverloads = function (type, A, N, I) {
return S;
};

module.exports.proveSimiliarity = function (ctx, overloads) {
let maxArguments = overloads[0].nameList.length;
for (let i = 1; i < overloads.length; ++i) {
if (overloads[i].nameList.length > maxArguments) {
maxArguments = overloads[i].nameList.length;
module.exports.distinguishingArgumentIndex = function (ctx, S) {
for (let i = 0; i < S[0].typeList.length; i++) {
let distinguishable = true;
for (let j = 0; j < S.length - 1; j++) {
for (let k = j + 1; k < S.length; k++) {
if (!areDistinguishable(ctx, S[j].typeList[i], S[k].typeList[i])) {
distinguishable = false;
}
}
}
}

const typeConversions = [];
for (let i = 0; i < maxArguments; ++i) {
if (overloads[0].operation.arguments.length <= i) {
break;
if (distinguishable) {
return i;
}

let maybeType = {
type: overloads[0].operation.arguments[i].idlType,
optional: overloads[0].optionalityList[i] !== "required",
default: overloads[0].operation.arguments[i].default
};
for (let j = 1; j < overloads.length; ++j) {
if (overloads[j].optionalityList[i] !== "required") {
maybeType.optional = true;
}

const thisType = overloads[j].operation.arguments[i].idlType;
if (maybeType.type.idlType !== thisType.idlType || maybeType.type.array !== thisType.array ||
maybeType.default !== overloads[j].operation.arguments[i].default) {
maybeType = null;
break;
for (let j = 0; j < S.length - 1; j++) {
for (let k = j + 1; k < S.length; k++) {
if (!sameType(ctx, S[j].typeList[i], S[k].typeList[i])) {
throw new Error(`Different but indistinguishable types at index ${i}`);
}
}
}

typeConversions.push(maybeType);
}

return typeConversions;
return -1;
};
Loading

0 comments on commit 21d86d0

Please sign in to comment.