Skip to content

Commit

Permalink
[idlharness.js] Simplify handling of inheritance and mixins
Browse files Browse the repository at this point in the history
Handling of these concepts was more complicated than (now) necessary.

Inheritance:

There's no reason to record inheritance separately in `IdlArray`'s
`this.["inheritance"]`, since it can be determined just as easily
from `this.members` via the `member.base` attributes.

The concept of "consequential interfaces" in Web IDL went away with
`implements` statements in whatwg/webidl#433
and here in #28619.
`traverse_inherited_and_consequential_interfaces()` can be replaced
with just `get_inheritance_stack()`.

Mixins:

For valid `A includes B` statements, `A` is always an interface and
`B` is always an interface mixin, so there are no include chains or
the possibility of cycles. `recursively_get_includes()` assumed this.

Instead just save the `includes` statements found in `this.includes`
and apply them in `merge_includes`, similar to partials.

The handling of partials isn't changed, but  `collapse_partials` is
renamed to `merge_partials` to match the above.

No observable differences in test results whatsoever are intended.
  • Loading branch information
foolip committed Apr 23, 2021
1 parent 161110d commit 185f655
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 170 deletions.
198 changes: 48 additions & 150 deletions resources/idlharness.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,27 +177,9 @@ self.IdlArray = function()
* might contain a partial interface or includes statement that depends
* on a later one. Save these up and handle them right before we run
* tests.
*
* .partials is simply an array of objects from WebIDLParser.js'
* "partialinterface" production. .includes maps strings to arrays of
* strings, such that
*
* A includes B;
* A includes C;
* D includes E;
*
* results in this["includes"] = { A: ["B", "C"], D: ["E"] }.
*
* Similarly,
*
* interface A : B {};
* interface B : C {};
*
* results in this["inheritance"] = { A: "B", B: "C" }
*/
this.partials = [];
this["includes"] = {};
this["inheritance"] = {};
this.includes = [];

/**
* Record of skipped IDL items, in case we later realize that they are a
Expand Down Expand Up @@ -249,11 +231,15 @@ IdlArray.prototype.internal_add_dependency_idls = function(parsed_idls, options)
const new_options = { only: [] }

const all_deps = new Set();
Object.values(this.inheritance).forEach(v => all_deps.add(v));
// NOTE: If 'A includes B' for B that we care about, then A is also a dep.
Object.keys(this.includes).forEach(k => {
all_deps.add(k);
this.includes[k].forEach(v => all_deps.add(v));
Object.values(this.members).forEach(v => {
if (v.base) {
all_deps.add(v.base);
}
});
// Add both 'A' and 'B' for each 'A includes B' entry.
this.includes.forEach(i => {
all_deps.add(i.target);
all_deps.add(i.includes);
});
this.partials.forEach(p => all_deps.add(p.name));
// Add 'TypeOfType' for each "typedef TypeOfType MyType;" entry.
Expand Down Expand Up @@ -434,11 +420,7 @@ IdlArray.prototype.internal_add_idls = function(parsed_idls, options)
{
return;
}
if (!(parsed_idl.target in this["includes"]))
{
this["includes"][parsed_idl.target] = [];
}
this["includes"][parsed_idl.target].push(parsed_idl["includes"]);
this.includes.push(parsed_idl);
return;
}

Expand All @@ -452,16 +434,6 @@ IdlArray.prototype.internal_add_idls = function(parsed_idls, options)
throw new IdlHarnessError("Duplicate identifier " + parsed_idl.name);
}

if (parsed_idl["inheritance"]) {
// NOTE: Clash should be impossible (would require redefinition of parsed_idl.name).
if (parsed_idl.name in this["inheritance"]
&& parsed_idl["inheritance"] != this["inheritance"][parsed_idl.name]) {
throw new IdlHarnessError(
`Inheritance for ${parsed_idl.name} was already defined`);
}
this["inheritance"][parsed_idl.name] = parsed_idl["inheritance"];
}

switch(parsed_idl.type)
{
case "interface":
Expand Down Expand Up @@ -529,34 +501,6 @@ IdlArray.prototype.prevent_multiple_testing = function(name)
this.members[name].prevent_multiple_testing = true;
};

IdlArray.prototype.recursively_get_includes = function(interface_name)
{
/**
* Helper function for test(). Returns an array of things that implement
* interface_name, so if the IDL contains
*
* A includes B;
* B includes C;
* B includes D;
*
* then recursively_get_includes("A") should return ["B", "C", "D"].
*/
var ret = this["includes"][interface_name];
if (ret === undefined)
{
return [];
}
for (var i = 0; i < this["includes"][interface_name].length; i++)
{
ret = ret.concat(this.recursively_get_includes(ret[i]));
if (ret.indexOf(ret[i]) != ret.lastIndexOf(ret[i]))
{
throw new IdlHarnessError("Circular includes statements involving " + ret[i]);
}
}
return ret;
};

IdlArray.prototype.is_json_type = function(type)
{
/**
Expand Down Expand Up @@ -753,37 +697,8 @@ IdlArray.prototype.test = function()
/** Entry point. See documentation at beginning of file. */

// First merge in all partial definitions and interface mixins.
this.collapse_partials();

for (var lhs in this["includes"])
{
this.recursively_get_includes(lhs).forEach(function(rhs)
{
var errStr = lhs + " includes " + rhs + ", but ";
if (!(lhs in this.members)) throw errStr + lhs + " is undefined.";
if (!(this.members[lhs] instanceof IdlInterface)) throw errStr + lhs + " is not an interface.";
if (!(rhs in this.members)) throw errStr + rhs + " is undefined.";
if (!(this.members[rhs] instanceof IdlInterface)) throw errStr + rhs + " is not an interface.";

if (this.members[rhs].members.length) {
test(function () {
var clash = this.members[rhs].members.find(function(member) {
return this.members[lhs].members.find(function(m) {
return this.are_duplicate_members(m, member);
}.bind(this));
}.bind(this));
this.members[rhs].members.forEach(function(member) {
assert_true(
this.members[lhs].members.every(m => !this.are_duplicate_members(m, member)),
"member " + member.name + " is unique");
this.members[lhs].members.push(new IdlInterfaceMember(member));
}.bind(this));
assert_true(!clash, "member " + (clash && clash.name) + " is unique");
}.bind(this), lhs + " includes " + rhs + ": member names are unique");
}
}.bind(this));
}
this["includes"] = {};
this.merge_partials();
this.merge_includes();

// Assert B defined for A : B
for (const member of Object.values(this.members).filter(m => m.base)) {
Expand Down Expand Up @@ -832,7 +747,7 @@ IdlArray.prototype.test = function()
}
};

IdlArray.prototype.collapse_partials = function()
IdlArray.prototype.merge_partials = function()
{
const testedPartials = new Map();
this.partials.forEach(function(parsed_idl)
Expand Down Expand Up @@ -926,6 +841,39 @@ IdlArray.prototype.collapse_partials = function()
this.partials = [];
}

IdlArray.prototype.merge_includes = function()
{
for (const parsed_idl of this.includes)
{
const lhs = parsed_idl.target;
const rhs = parsed_idl.includes;

var errStr = lhs + " includes " + rhs + ", but ";
if (!(lhs in this.members)) throw errStr + lhs + " is undefined.";
if (!(this.members[lhs] instanceof IdlInterface)) throw errStr + lhs + " is not an interface.";
if (!(rhs in this.members)) throw errStr + rhs + " is undefined.";
if (!(this.members[rhs] instanceof IdlInterface)) throw errStr + rhs + " is not an interface.";

if (this.members[rhs].members.length) {
test(function () {
var clash = this.members[rhs].members.find(function(member) {
return this.members[lhs].members.find(function(m) {
return this.are_duplicate_members(m, member);
}.bind(this));
}.bind(this));
this.members[rhs].members.forEach(function(member) {
assert_true(
this.members[lhs].members.every(m => !this.are_duplicate_members(m, member)),
"member " + member.name + " is unique");
this.members[lhs].members.push(new IdlInterfaceMember(member));
}.bind(this));
assert_true(!clash, "member " + (clash && clash.name) + " is unique");
}.bind(this), lhs + " includes " + rhs + ": member names are unique");
}
}
this.includes = [];
}

IdlArray.prototype.are_duplicate_members = function(m1, m2) {
if (m1.name !== m2.name) {
return false;
Expand Down Expand Up @@ -1416,7 +1364,7 @@ IdlInterface.prototype.get_inheritance_stack = function() {
*/
IdlInterface.prototype.default_to_json_operation = function(callback) {
var map = new Map(), isDefault = false;
this.traverse_inherited_and_consequential_interfaces(function(I) {
this.get_inheritance_stack().forEach(function(I) {
if (I.has_default_to_json_regular_operation()) {
isDefault = true;
I.members.forEach(function(m) {
Expand All @@ -1431,56 +1379,6 @@ IdlInterface.prototype.default_to_json_operation = function(callback) {
return isDefault ? map : null;
};

/**
* Traverses inherited interfaces from the top down
* and imeplemented interfaces inside out.
* Invokes |callback| on each interface.
*
* This is an abstract implementation of the traversal
* algorithm specified in:
* https://heycam.github.io/webidl/#collect-attribute-values
* Given the following inheritance tree:
*
* F
* |
* C E - I
* | |
* B - D
* |
* G - A - H - J
*
* Invoking traverse_inherited_and_consequential_interfaces() on A
* would traverse the tree in the following order:
* C -> B -> F -> E -> I -> D -> A -> G -> H -> J
*/

IdlInterface.prototype.traverse_inherited_and_consequential_interfaces = function(callback) {
if (typeof callback != "function") {
throw new TypeError();
}
var stack = this.get_inheritance_stack();
_traverse_inherited_and_consequential_interfaces(stack, callback);
};

function _traverse_inherited_and_consequential_interfaces(stack, callback) {
var I = stack.pop();
callback(I);
var mixins = I.array["includes"][I.name];
if (mixins) {
mixins.forEach(function(id) {
var mixin = I.array.members[id];
if (!mixin) {
throw new Error("Interface mixin " + id + " not found (included by " + I.name + ")");
}
var interfaces = mixin.get_inheritance_stack();
_traverse_inherited_and_consequential_interfaces(interfaces, callback);
});
}
if (stack.length > 0) {
_traverse_inherited_and_consequential_interfaces(stack, callback);
}
}

IdlInterface.prototype.test = function()
{
if (this.has_extended_attribute("LegacyNoInterfaceObject") || this.is_mixin())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
idlArray.add_idls(`
partial dictionary C {};
namespace C {};`);
idlArray.collapse_partials();
idlArray.merge_partials();
})();
</script>
<script type="text/json" id="expected">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,56 @@
<script>
"use strict";

// No original existence
// Simple includes statement (valid)
(() => {
const idlArray = new IdlArray();
idlArray.add_idls('partial interface mixin A {};');
idlArray.test();
idlArray.add_idls(`
[Exposed=Window] interface I1 {};
interface mixin M1 { attribute any a1; };
I1 includes M1;`);
idlArray.merge_partials();
idlArray.merge_includes();
})();

// Partial interface mixin (valid)
(() => {
const idlArray = new IdlArray();
idlArray.add_idls(`
[Exposed=Window] interface I2 {};
interface mixin M2 {};
partial interface mixin M2 { attribute any a2; };
I2 includes M2;`);
idlArray.merge_partials();
idlArray.merge_includes();
})();

// Partial interface mixin without original mixin
(() => {
const idlArray = new IdlArray();
idlArray.add_idls('partial interface mixin M3 {};');
idlArray.merge_partials();
idlArray.merge_includes();
})();

// Name clash (partials)
// Name clash between mixin and partial mixin
(() => {
const idlArray = new IdlArray();
idlArray.add_idls(`
interface mixin B { attribute any F; };
partial interface mixin B { attribute any F; };`);
idlArray.collapse_partials();
interface mixin M3 { attribute any a3; };
partial interface mixin M3 { attribute any a3; };`);
idlArray.merge_partials();
idlArray.merge_includes();
})();

// Name clash (different mixins)
// Name clash between interface and mixin
(() => {
const idlArray = new IdlArray();
idlArray.add_untested_idls(`
interface mixin C { attribute any F; };
interface D { attribute any F; };
D includes C;`);
idlArray.test();
interface mixin M4 { attribute any a4; };
interface I4 { attribute any a4; };
I4 includes I4;`);
idlArray.merge_partials();
idlArray.merge_includes();
})();
</script>
<script type="text/json" id="expected">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
[Exposed=(Worker)]
partial interface C {};`);
idlArray.collapse_partials();
idlArray.merge_partials();
})();

// Invalid exposure
Expand All @@ -51,7 +51,7 @@
idlArray.add_idls(`
[Exposed=(DedicatedWorker)]
partial interface D {};`);
idlArray.collapse_partials();
idlArray.merge_partials();
})();

// Original is a namespace, not an interface.
Expand All @@ -60,7 +60,7 @@
idlArray.add_idls(`
partial interface E {};
namespace E {};`);
idlArray.collapse_partials();
idlArray.merge_partials();
})();

(() => {
Expand Down Expand Up @@ -100,7 +100,7 @@
idlArray.add_idls(`
interface M { attribute any A; };
partial interface M { attribute any A; };`);
idlArray.collapse_partials();
idlArray.merge_partials();
})();
</script>
<script type="text/json" id="expected">
Expand Down
Loading

0 comments on commit 185f655

Please sign in to comment.