Re: [heycam/webidl] Normative: Match ECMA‑262 function property enumeration order (#914)

@TimothyGu commented on this pull request.

Just wondering @ExE-Boss, have you had any progress on writing tests for this PR?

> @@ -11792,11 +11786,8 @@ in which case they are exposed on every object that [=implements=] the interface
         1.  If |attribute|'s type is a [=promise type=], then return
             [=!=] <a abstract-op>Call</a>({{%Promise.reject%}}, {{%Promise%}}, «|E|»).
         1.  Otherwise, end these steps and allow the exception to propagate.
-    1.  Let |F| be [=!=] <a abstract-op>CreateBuiltinFunction</a>(|steps|, « », |realm|).
-    1.  Let |name| be the string "<code>get </code>" prepended to |attribute|'s [=identifier=].
-    1.  Perform [=!=] <a abstract-op>SetFunctionName</a>(|F|, |name|).
-    1.  Perform [=!=] <a abstract-op>SetFunctionLength</a>(|F|, 0).
-    1.  Return |F|.
+    1.  Let |name| be |attribute|'s [=identifier=].
+    1.  Return [=!=] [$CreateBuiltinFunction$](|steps|, 0, |name|, « », |realm|, {{%Function.prototype%}}, "<code>get</code>").

```suggestion
    1.  Return [=!=] [$CreateBuiltinFunction$](|steps|, 0, |name|, « », |realm|,
        |realm|.\[[Intrinsics]].\[[{{%Function.prototype%}}]], "<code>get</code>").
```

We need to use the %Function.prototype% from _realm_, since when this operation is called there could well be no current realm.

> @@ -11878,11 +11869,8 @@ in which case they are exposed on every object that [=implements=] the interface
         1.  Perform the [=setter steps=] of |attribute|,
             with |idlObject| as [=this=] and |idlValue| as [=the given value=].
         1.  Return <emu-val>undefined</emu-val>
-    1.  Let |F| be [=!=] <a abstract-op>CreateBuiltinFunction</a>(|steps|, « », |realm|).
-    1.  Let |name| be the string "<code>set </code>" prepended to |id|.
-    1.  Perform [=!=] <a abstract-op>SetFunctionName</a>(|F|, |name|).
-    1.  Perform [=!=] <a abstract-op>SetFunctionLength</a>(|F|, 1).
-    1. Return |F|.
+    1.  Let |name| be |attribute|'s [=identifier=].
+    1.  Return be [=!=] [$CreateBuiltinFunction$](|steps|, 1, |name|, « », |realm|, {{%Function.prototype%}}, "<code>set</code>").

Ditto. Explicitly get %Function.prototype% from _realm_.

>      1.  Perform [=!=] [$CreateDataProperty$](|handler|, "<code>preventExtensions</code>", |preventExtensions|).
-    1.  Let |set| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-set]], « », |realm|).
+    1.  Let |set| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-set]], the number of non-optional parameters from [[#es-observable-array-set]], "<code>set</code>", « », |realm|).
     1.  Perform [=!=] [$CreateDataProperty$](|handler|, "<code>set</code>", |set|).

For readability, may I suggest a tabular format like this:

6. For each row in the following table:

   Name | Section | Length
   ---|---|---
   "`defineProperty`" | [§ 3.10.1 defineProperty](https://heycam.github.io/webidl/#es-observable-array-defineProperty) | 3
   "`deleteProperty`" | [§ 3.10.2 deleteProperty](https://heycam.github.io/webidl/#es-observable-array-deleteProperty) | 2
   … | … | …

   1. Let (_name_, _section_, _length_) be the entries of the current row.
   2. Let _F_ be ! CreateBuiltinFunction(the steps from _section_, _length_, _name_, « », _realm_).
   3. Perform ! CreateDataProperty(_handler_, _name_, _F_).

This helps make length changes more visible to implementors and folks writing tests, and also seems to be generally more readable.

-- 
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/914#pullrequestreview-695578515