Ecma262: NamedEvaluation calls may require ReturnIfAbrupt

[TimothyGu]

Currently, none of the calls to the NamedEvaluation runtime semantic use ReturnIfAbrupt, but in many of these cases, ReturnIfAbrupt is necessary. See for example https://tc39.es/ecma262/#sec-object-initializer-runtime-semantics-propertydefinitionevaluation

_PropertyDefinition_ : _PropertyName_ : _AssignmentExpression_

rule: the result of performing NamedEvaluation is directly passed into CreateDataPropertyOrThrow, which must take a value as _propValue_ rather than a Completion Record. This means code fragments such as

({ a: class extends (() => { throw 'b'; })() {} })

would currently break the spec (and engine262).

I've compiled a list of such instances:

• https://tc39.es/ecma262/#sec-exports-runtime-semantics-evaluation
• https://tc39.es/ecma262/#sec-object-initializer-runtime-semantics-propertydefinitionevaluation
• https://tc39.es/ecma262/#sec-__proto__-property-names-in-object-initializers

/cc @jmdyck

[ljharb]

What do engines do here?

[TimothyGu]

Engines all throw “b” I recall.

[devsnek]

eshost output is this:

$ eshost -ts -x "({ a: class extends (() => { throw 'b'; })() {} }); print(1);
┌────────────────┬──────────────┐
│ ChakraCore     │              │
│ engine262      │ b: undefined │
│ engine262+     │              │
│ JavaScriptCore │              │
│ V8             │              │
├────────────────┼──────────────┤
│ SpiderMonkey   │              │
│ XS             │              │
└────────────────┴──────────────┘

@rwaldron xs and sm do throw here, but the output seems to be missing

[allenwb]

All invocations of NamedEvaluation are guarded by an expression like:

   If IsAnonymousFunctionDefinition(AssignmentExpression) is true, then

When this pattern was first used in draft ES6, NamedEvaluation is only called when the _AssignmentExpression_ (or _Initializer_ in most other occurrences) consists entirely of a literal definition of a function. NamedEvaluation of such a literal definition did return an abrupt completion so there is no need to check for it.

This was either before class definitions were introduced or perhaps the cases of evaluative extends and computed property keys were overlooked. Regardless its a spec bug! All calls to NamedEvaluation now need to have a ?.

[jmdyck]

@TimothyGu:

I've compiled a list of such instances:

• https://tc39.es/ecma262/#sec-object-initializer-runtime-semantics-propertydefinitionevaluation
• https://tc39.es/ecma262/#sec-exports-runtime-semantics-evaluation
• https://tc39.es/ecma262/#sec-__proto__-property-names-in-object-initializers

Yup, also:

• https://tc39.es/ecma262/#sec-runtime-semantics-iteratordestructuringassignmentevaluation
• https://tc39.es/ecma262/#sec-runtime-semantics-keyeddestructuringassignmentevaluation

(I think.)

@allenwb:

All calls to NamedEvaluation now need to have a ?.

In some cases, it's optional (editors' choice). E.g. the ones in steps of the form
Return the result of performing NamedEvaluation ...
and the ones where the result is (only) passed to an operation that handles abrupts, like PutValue or InitializeReferencedBinding.

[TimothyGu]

@jmdyck In the other two cases you identified, you actually don't need a ?. This is because NamedEvaluation would only happen if _DestructuringAssignmentTarget_ is an identifier (IsIdentifierRef), but then it cannot be an _ObjectLiteral_ or an _ArrayLiteral_, so it would always go through the PutValue.

[jmdyck]

Ah, I was wondering if it was something like that. In such cases, where it isn't immediately obvious that an abrupt result will be properly handled, there's a risk that a future insertion might attempt to use the value without realizing that it might be abrupt, so I'd support a ? at the point where it's introduced.

[allenwb]

and the ones where the result is (only) passed to an operation that handles abrupts, like PutValue or

I did quite a bit of that sort of deferral in ES6 because it took a separate step to do a ReturnIfAbrupt. With the ? operator available, I think it is clearer and more maintainable to always use it. Implementations can prove for themselves that in certain cases an abrupt completion will never occur. However, if any of this is changed in the spec you will need to make sure that you aren't moving the check past a observable side-effect producing operation.