Re: [heycam/webidl] Revamp interface bindings (#283)

domenic commented on this pull request.

This is really, really great modernization and bug-fixing work. Found some nits, but overall quite happy.

>  that is the value of the aforementioned property.
 
-The identifier used for the named constructor must not
+The [=NamedConstructor identifier|identifier=] used for the named constructor must not

I could go either way. We don't have separate #x-identifier concepts for everything else. On the other hand, the "after the = sign" thing down below does sure seem out of place, so moving it up here is not a bad idea.

>  
-    1.  Let |id| be the identifier of interface |I|.
-    1.  Initialize |S| to the
-        [=effective overload set=]
-        for constructors with
-        [=identifier=]
-        |id| on [=interface=]
-        |I| and with argument count 0.
-    1.  Return the length of the shortest argument list of the entries in |S|.
+    1.  Let |steps| be the following steps:
+        1.  If |I| was not declared with a [{{Constructor}}] [=extended attribute=],
+            then [=ECMAScript/throw=] a <emu-val>TypeError</emu-val>.
+        1.  If [=NewTarget=] is <emu-val>undefined</emu-val>, then
+            [=ECMAScript/throw=] a <emu-val>TypeError</emu-val>.
+        1.  Let |arg|<sub>0..|n|−1</sub> be arguments.

"the passed arguments" (here and below)

> +            on [=interface=] |I| and with argument count |n|.
+        1.  Let &lt;|constructor|, |values|&gt; be the result
+            of passing |S| and |arg|<sub>0..|n|−1</sub>.
+            to the [=overload resolution algorithm=].
+        1.  Let |R| be the result of performing
+            the actions listed in the description of |constructor|
+            with |values| as the argument values.
+            Rethrow any exceptions.
+        1.  Let |O| be the result of [=converted to an ECMAScript value|converting=] |R|
+            to an ECMAScript [=interface type=] value |I|.
+        1.  Assert: |O| is an object that implements |I|.
+        1.  Assert: |O|.\[[Realm]] is equal to |F|.\[[Realm]].
+        1.  Return |O|.
+    1.  Let |proto| be the [=%FunctionPrototype%=] of |realm|.
+    1.  If |I| inherits from some other interface |P|,
+        then set |proto| to the [=interface object=] of |P|.

"interface object of P in _realm_".

>  
-    Interface objects for non-callback interfaces must have a property named “length” with attributes
-    { \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }
-    whose value is a <emu-val>Number</emu-val>.
-    If the [{{Constructor}}] [=extended attribute=]
-    does not appear on the interface definition, then the value is 0.
-    Otherwise, the value is determined as follows:
+    The [=interface object=]
+    for a given non-callback [=interface=] |I| with [=identifier=] |id|
+    and [=Realm=] |realm| is created as follows:

"and in Realm realm"

> +    1.  If |I| inherits from some other interface |P|,
+        then set |proto| to the [=interface object=] of |P|.
+    1.  Let |F| be [=!=] [=CreateBuiltinFunction=](|realm|, |steps|, |proto|).
+    1.  Perform [=!=] [=SetFunctionName=](|F|, |id|).
+    1.  Let |length| be 0.
+    1.  If |I| was declared with a [{{Constructor}}] [=extended attribute=], then
+        1.  Initialize |S| to the [=effective overload set=]
+            for constructors with [=identifier=] |id| on [=interface=] |I| and
+            with argument count 0.
+        1.  Set |length| to the length of the
+            shortest argument list of the entries in |S|.
+    1.  Perform [=!=] [=DefinePropertyOrThrow=](|F|, "length",
+        PropertyDescriptor{\[[Value]]: |length|, \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val>}).
+    1.  Let |obj| be the [=interface prototype object=] of [=interface=] |I|.
+    1.  Perform [=!=] [=DefinePropertyOrThrow=](|F|, "prototype",
+        PropertyDescriptor{\[[Value]]: |obj|, \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>false</emu-val>}).

s/obj/proto; surprised the var checker didn't catch this

> -        |arg|<sub>0..|n|−1</sub> to the
-        [=overload resolution algorithm=].
-    1.  Let |R| be the result of performing the actions listed in the description of
-        |constructor| with |values| as the argument values.
-    1.  Return the result of [=converted to an ECMAScript value|converting=]
-        |R| to an ECMAScript [=interface type=] value |I|.
-</div>
-
-If the internal \[[Call]] method
-of the [=named constructor=]
-returns normally, then it must
-return an object that implements interface |I|.
-This object also must be
-associated with the ECMAScript global environment associated
-with the [=named constructor=].
+If the internal \[[Call]] method of the [=named constructor=] returns normally,

Prexisting problem, but: this shouldn't be talking about the internal [[Call]] method. It should be talking about the steps that spec authors use to describe the named constructor ("the actions listed in the description of constructor" below).

> @@ -10141,87 +10093,67 @@ which allows construction of objects that
 implement the interface on which the
 [{{NamedConstructor}}] extended attributes appear.

I would remove the "must have a [[Call]] property" sentence; that is implicit and restating it is confusing (how could it not be true)?

>  
-    A named constructor must have a property named “length” with attributes
-    { \[[Writable]]: <emu-val>false</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }
-    whose value is a <emu-val>Number</emu-val> determined as follows:
+    Assuming |id| is the [=NamedConstructor identifier|identifier=] of the constructor

I'd restate this intro as you did for interface objects. E.g.

> The named constructor for a given non-callback interface I with identifier id in Realm realm is created as follows:

>  
-If the [{{NoInterfaceObject}}]
-extended attribute was not specified on the interface, then
-the interface prototype object must
-also have a property named “constructor” with attributes
-{ \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> } whose value
-is a reference to the interface object for the interface.
+If the [{{NoInterfaceObject}}] extended attribute was not specified on the interface,
+then the interface prototype object must also
+have a property named “constructor” with attributes
+{ \[[Writable]]: <emu-val>true</emu-val>, \[[Enumerable]]: <emu-val>false</emu-val>, \[[Configurable]]: <emu-val>true</emu-val> }
+whose value is a reference to the interface object for the interface.

Link interface object while you're here?

> +is a [=function object=].
+It has properties that correspond to the [=constants=] defined on that interface,
+as described in sections [[#es-constants]].
+
+Note: Since a legacy callback interface object is a [=function object=]
+the <code>typeof</code> operator will return "function"
+when applied to a [=legacy callback interface object=].
+
+<div algorithm="to create a legacy callback interface object">
+
+    The [=legacy callback interface object=]
+    for a given [=callback interface=] with [=identifier=] |id|
+    and [=Realm=] |realm| is created as follows:
+
+    1.  Let |steps| be the following steps:
+        1.  [=ECMAScript/throw=] a <emu-val>TypeError</emu-val>.

Capitalize throw

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/283#pullrequestreview-18703557