Closed Bug 1178932 Opened 7 years ago Closed 7 years ago

Implement ES6 Reflect.construct

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: jorendorff, Assigned: jorendorff)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: [DocArea=JS])

Attachments

(1 file, 2 obsolete files)

No description provided.
Attached patch Implement Reflect.construct (obsolete) — Splinter Review
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Eric, would this help you test super()?

If so, steal reviews in bug 987514 and bug 1170307 and I'll land it today.
Flags: needinfo?(efaustbmo)
Hmm. I think that even though this is useful for "recreating the same circumstances", we should still test super() in its native habitat. It's definitely a good tool for reducing crashes and doing manual translation to remove boilerplate, but I would feel weird reading "super()" tests based on R.c. That being said, it would be cool to have, so I might steal the reviews anyways.
Flags: needinfo?(efaustbmo)
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
Attached patch Implement Reflect.construct (obsolete) — Splinter Review
Attachment #8634531 - Flags: review?(efaustbmo)
Attachment #8627837 - Attachment is obsolete: true
Attachment #8634534 - Flags: review?(efaustbmo)
Attachment #8634531 - Attachment is obsolete: true
Attachment #8634531 - Flags: review?(efaustbmo)
Comment on attachment 8634534 [details] [diff] [review]
Implement Reflect.construct

Review of attachment 8634534 [details] [diff] [review]:
-----------------------------------------------------------------

Take it if you want it, to land earlier (even in post-time-readiness).

::: js/src/tests/ecma_6/Reflect/argumentsList.js
@@ +49,3 @@
>  
>  // argumentList.length can be more than func.length.
>  function testMoar(a) {

Lulz, you actually did rename it.  :-D

::: js/src/tests/ecma_6/Reflect/construct.js
@@ +22,5 @@
> +// JS functions:
> +function f(x) { this.x = x; }
> +assertDeepEq(Reflect.construct(f, [3]), new f(3));
> +f.prototype = Array.prototype;
> +assertDeepEq(Reflect.construct(f, [3]), new f(3));

Also assert inequality of the old |new f(3)| and the new |new f(3)|'s prototypes?  Or their equality to the initial f.prototype and to Array.prototype respectively.

@@ +26,5 @@
> +assertDeepEq(Reflect.construct(f, [3]), new f(3));
> +
> +// Bound functions:
> +var bound = f.bind(null, "carrot");
> +assertDeepEq(Reflect.construct(bound, []), new bound);

Also assert deep equality with new f("carrot")?

@@ +54,5 @@
> +
> +        assertDeepEq(Reflect.construct(Base, []), new Base);
> +        //assertDeepEq(Reflect.construct(Derived, [7]), new Derived(7));
> +        //g = Derived.bind(null, "q");
> +        //assertDeepEq(Reflect.construct(g, [8, 9]), new g(8, 9));

Compare against new Derived("q", 8, 9) as well?

@@ +96,5 @@
> +    assertEq(new.target, expected);
> +    expected = undefined;
> +}
> +var expected = checkNewTarget;
> +Reflect.construct(checkNewTarget, []);

assertEq(expected, undefined) as verification, as below?
Attachment #8634534 - Flags: review+
Comment on attachment 8634534 [details] [diff] [review]
Implement Reflect.construct

Thanks, Jeff!
Attachment #8634534 - Flags: review?(efaustbmo)
https://hg.mozilla.org/mozilla-central/rev/e673cc171604
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.