Throw when not calling interfaces as constructors #205

tobie · 2016-10-25T16:29:51Z

Fixes #62.
Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=22808.

Preview | Diff w/ current ED | Diff w/ base

domenic · 2016-10-25T17:05:21Z

index.bs


 <ol class="algorithm">
    1.  If |I| was not declared with a [{{Constructor}}]
        [=extended attribute=], then
        <a lt="es throw">throw a <emu-val>TypeError</emu-val></a>.
+    1.  If |newTarget| is <emu-val>undefined</emu-val>, then


ES notation is just "NewTarget", not "|newTarget|".

Well, it gets passed to the algorithm, no? So that's just its local name.

Yeah. I guess just remove that "and |newTarget| as the NewTarget value" to conform to ES functions.

Wouldn't that just turn NewTarget into a free variable though?

Seems the ES spec treats it as a local variable in some place, e.g.: https://tc39.github.io/ecma262/#sec-construct

I don't understand the advice from @domenic. It seems ES also always introduces it as a variable of an operation in the prose preceding the steps.

Inside built-in function definitions (which this is one of), NewTarget is an ambient value, like "this value" is.

Oh, indeed, in section 17:

In the description of a particular function, the terms “this value” and “NewTarget” have the meanings given in 9.3.

Should we maybe link the term to section 9.3?

Personally I've been holding out for tc39/ecma262#529, but it wouldn't hurt I guess to link to that section for now.

In tc39/ecma262#529 (comment), @allenwb writes:

"NewTarget" and "this value" should only be used in the top-level algorithms that defined built-ins. They should not be used in other abstract operations (because paragraph 3 of 9.3 doesn't generally apply to abstract operations). If an abstract operation that is used to factor the definition of one or more built-ins need to access those values they should be explicitly passed as parameters to the abstract operation (for example see https://tc39.github.io/ecma262/#sec-regexpalloc).

Aren't we in the case described by that last sentence, here?

No. This is a top-level algorithm defining a built-in (namely, the built-in constructor for all platform objects).

domenic · 2016-10-25T17:06:57Z

index.bs

-returns normally, then it must
-return an object that implements interface |I|.
+If evaluating the [=function object=] |F| returns normally,
+then it must return an object that implements interface |I|.


Hmm. This requirement is a little strange, both before and after. I think what it is trying to say is instead "If the actions listed in the description of constructor return normally, then they must return an object that implements interface I."

Oh! Are you saying the before-last step of the above algorithm should be something like this:

1. Assert |R| is an object that implements |I|

Added that as an Assert: statement, which might be totally dumb. LMK.

domenic · 2016-10-25T17:09:59Z

index.bs

-|arg|<sub>0..|n|−1</sub> is the list
-of argument values passed to the constructor, and |I|
-is the [=interface=]:
+Interfaces that do not have a constructor will throw


Maybe "interfaces that are not declared with a [Constructor] extended attribute will throw when called, both as a function and as a constructor."

bzbarsky · 2016-10-25T19:56:03Z

index.bs

 This object also must be
 associated with the ECMAScript global environment associated
-with the interface object.
+with |I|.


This isn't right. I is an interface. It has no global environment associated with it. Do you mean the global environment associated with F?

I guess maybe just fix #206 at the same time.

Bundled that up as an Assert: statement. You might have had something completely different in mind—LMK.

Also unsure if said assert should operate on |R| or on the result of converting |R| to ES (last step of the algo).

domenic

I like the asserts. Just minor things left.

domenic · 2016-10-26T01:19:15Z

index.bs

-|arg|<sub>0..|n|−1</sub> is the list
-of argument values passed to the constructor, and |I|
-is the [=interface=]:
+Interfaces that are not declared with a [Constructor] extended attribute will throw when called,


Link [{{Constructor}}]

domenic · 2016-10-26T01:23:12Z

index.bs

@@ -10361,19 +10363,12 @@ is the [=interface=]:
        [=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.  Assert: |R| is an object that implements |I|.


Hmm. I think at this point |R| is a completion value, so this doesn't quite work. You'll want to add a step before the asserts that simply says "ReturnIfAbrupt(R)."

For further clarity, maybe add ", as a [=completion value=]" to the "Let R be the result..." step.

There's something I don't quite understand here. If |R| is a completion value, then |R|.[[Value]] is an ECMAScript language value. If so, why do we "convert |R| to an ECMAScript interface type value |I|" in the the last step of the algorithm?

R is something that's not concretely defined since we don't have requirements for what IDL-prose can and cannot be. I agree that it cannot really be a completion value, but I also agree with @domenic that it can have thrown an exception (constructors do this) and we do need to handle that somehow.

I guess we just need to handle this in prose, then.

Good point, yeah. Instead maybe let's add "Rethrow any exceptions." to the previous step.

Reorganized some of these last steps. LMK if it makes sense.

domenic · 2016-11-03T15:30:37Z

index.bs

@@ -10316,18 +10318,14 @@ an exception when called as a function.
        [=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=]
+    1.  Rethrow any [=exception=] thrown in the previous step.


I don't think this is a Web IDL exception, so the linking here seems bad.

Also, I think we should inline this into the previous step. It's a single atomic operation; we're just stating explicitly what to do if any exceptions occur.

Can't constructors throw IDL errors like SecurityError?

Sure, but they can also throw anything else.

Don't simple exceptions englobe all of the ES errors that can be thrown at runtime?

You can throw anything. E.g., throw 2. Not sure if a constructor ever does that, but it might with custom elements I suppose.

I have no words. o_O

Not sure if a constructor ever does that

It doesn't even have to be the constructor itself. For example, step 4 (invoking the overload resolution algorithm) can trigger arbitrary page-provided code which can throw whatever the page wants. In general, so can step 5, if the constructor takes an object argument and pokes at it in any way.

Fixes whatwg#62. Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=22808.

domenic approved these changes Oct 25, 2016

View reviewed changes

domenic mentioned this pull request Oct 25, 2016

State the associated realm when creating objects #206

Closed

bzbarsky reviewed Oct 25, 2016

View reviewed changes

domenic reviewed Oct 26, 2016

View reviewed changes

tobie force-pushed the new-target branch from ea21993 to 76512d1 Compare November 3, 2016 10:12

domenic reviewed Nov 3, 2016

View reviewed changes

domenic approved these changes Nov 3, 2016

View reviewed changes

Throw when not calling interfaces as constructors

208dbe3

Fixes whatwg#62. Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=22808.

tobie force-pushed the new-target branch from 8d73fda to 208dbe3 Compare November 3, 2016 21:10

tobie merged commit 001ba52 into whatwg:gh-pages Nov 3, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Throw when not calling interfaces as constructors #205

Throw when not calling interfaces as constructors #205

tobie commented Oct 25, 2016 •

edited

Loading

domenic Oct 25, 2016

tobie Oct 25, 2016

domenic Oct 26, 2016

tobie Oct 27, 2016

annevk Oct 27, 2016

domenic Oct 27, 2016

tobie Oct 27, 2016

domenic Oct 27, 2016

tobie Nov 3, 2016

domenic Nov 3, 2016

domenic Oct 25, 2016

tobie Oct 25, 2016

tobie Oct 25, 2016

domenic Oct 25, 2016

bzbarsky Oct 25, 2016

domenic Oct 25, 2016

tobie Oct 25, 2016 •

edited

Loading

domenic left a comment

domenic Oct 26, 2016

domenic Oct 26, 2016

tobie Oct 27, 2016

annevk Oct 27, 2016

tobie Oct 27, 2016

domenic Oct 27, 2016

tobie Nov 3, 2016

domenic Nov 3, 2016

domenic Nov 3, 2016

tobie Nov 3, 2016 •

edited

Loading

domenic Nov 3, 2016

tobie Nov 3, 2016

annevk Nov 3, 2016

tobie Nov 3, 2016

bzbarsky Nov 3, 2016

Throw when not calling interfaces as constructors #205

Throw when not calling interfaces as constructors #205

Conversation

tobie commented Oct 25, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobie Oct 25, 2016 • edited Loading

Choose a reason for hiding this comment

domenic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobie Nov 3, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobie commented Oct 25, 2016 •

edited

Loading

tobie Oct 25, 2016 •

edited

Loading

tobie Nov 3, 2016 •

edited

Loading