CreateImmutableBinding called with only one argument

[erights]

At https://tc39.es/ecma262/#sec-asyncgenerator-definitions-evaluation
and https://tc39.es/ecma262/#sec-async-function-definitions-runtime-semantics-evaluation

for the Async*Expression cases where there's a Bindingidentifier, step 5 is

5. Perform ! envRec.CreateImmutableBinding(name).

By analogy with FunctionExpression https://tc39.es/ecma262/#sec-function-definitions-runtime-semantics-evaluation and GeneratorExpression https://tc39.es/ecma262/#sec-generator-function-definitions-runtime-semantics-evaluation It should be

5. Perform ! envRec.CreateImmutableBinding(name, false).

[ljharb]

https://tc39.es/ecma262/#sec-declarative-environment-records-createimmutablebinding-n-s does implicitly assert that the arg is a Boolean and not optional, but only cares if it’s true. This seems like a good thing to tighten up, but not a spec bug persay.

[erights]

I don't understand how it can be not a spec bug. The spec violates its own consistency rules, and so is not well formed.

[ljharb]

What i mean is, i can’t conceive how anybody could have implemented it incorrectly, since “false” and “not present” (implicitly undefined for abstract ops, per spec) are both not “true”.

[jmdyck]

What i mean is, i can’t conceive how anybody could have implemented it incorrectly,

Agreed.

“not present” (implicitly undefined for abstract ops, per spec)

Citation? That's for functions, not abstract ops.

And even if there were such a rule, it wouldn't apply, because there's nothing to indicate that CreateImmutableBinding's second parameter is optional.

[ljharb]

Right, that’s what needs fixing - either adding the false or making it optional.

On reflection i see that it is indeed a spec bug.

[allenwb]

In the ES6 spec Table 15 said that the second argument to CreateImmutableBinding is optional and defaults to false

[jmdyck]

That went away when PR #430 (9e15064) was merged. That PR also changed the existing 1-arg invocations to 2-args. New 1-arg invocations were added in PR #692 (869f1c6) and PR #1066 (b564aa9).

[erights]

In the ES6 spec Table 15 said that the second argument to CreateImmutableBinding is optional and defaults to false

I don't see any reason to complexify internal-only operations with optional parameters. Are there any others?

[ljharb]

There’s a few, but in this case it seems clear that the reintroductions were based on a spec that predates the change to make it required, so the bug was in making them optional. We should just add the missing falses, i think.

[erights]

What are the other internal-only operations with optional parameters?

[ljharb]

https://tc39.es/ecma262/#sec-getiterator, https://tc39.es/ecma262/#sec-iteratornext, https://tc39.es/ecma262/#sec-toprimitive, https://tc39.es/ecma262/#sec-call, https://tc39.es/ecma262/#sec-construct, https://tc39.es/ecma262/#sec-createlistfromarraylike, https://tc39.es/ecma262/#sec-createlistfromarraylike, to name a few.

[erights]

I see. Yes, in general worth it. Thanks.

[allenwb]

For some background, I believe that originally CreateImmutableBinding only had one argument and that the second argument was added to support a special case that was identified later. I don't recall specifically, but that special case probably only had a single "call site" that need to trigger it with the second argument. In that case I would have made it optional to avoid churn in algorithms that already used the one argument form (partially to minimize spec maintanance over head and partially to avoid signalling implementors that there was a meaningful change to the existing algorithms).

Given the existing world of multiple proposal streams, multiple implementations, and multiple editors I would think that such churn issues would be an even bigger concern.

[jmdyck]

I don't see any reason to complexify internal-only operations with optional parameters.

I'm inclined to agree, especially when omitting the argument has a semantics that could easily and briefly be supplied explicitly instead (e.g., an empty list). But I don't feel strongly enough to propose de-optionalizing the ones we currently have.

Are there any others?

I think this is a complete list (25):

• 7.1.1 ToPrimitive ( input [ , PreferredType ] )
• 7.3.12 Call ( F, V [ , argumentsList ] )
• 7.3.13 Construct ( F [ , argumentsList [ , newTarget ] ] )
• 7.3.18 CreateListFromArrayLike ( obj [ , elementTypes ] )
• 7.3.19 Invoke ( V, P [ , argumentsList ] )
• 7.4.1 GetIterator ( obj [ , hint [ , method ] ] )
• 7.4.2 IteratorNext ( iteratorRecord [ , value ] )
• 8.3.2 ResolveBinding ( name [ , env ] )
• 9.1.12 ObjectCreate ( proto [ , internalSlotsList ] )
• 9.1.13 OrdinaryCreateFromConstructor ( constructor, intrinsicDefaultProto [ , internalSlotsList ] )
• 9.2.5 MakeConstructor ( F [ , writablePrototype [ , prototype ] ] )
• 9.2.8 SetFunctionName ( F, name [ , prefix ] )
• 9.3.3 CreateBuiltinFunction ( steps, internalSlotsList [ , realm [ , prototype ] ] )
• 9.4.2.2 ArrayCreate ( length [ , proto ] )
• 13.7.5.13 ForIn/OfBodyEvaluation ( lhs, stmt, iteratorRecord, iterationKind, lhsKind, labelSet [ , iteratorKind ] )
• 15.2.1.17.2 GetExportedNames ( [ exportStarSet ] ) Concrete Method
• 15.2.1.17.3 ResolveExport ( exportName [ , resolveSet ] ) Concrete Method
• 22.1.3.10.1 FlattenIntoArray ( target, source, sourceLen, start, depth [ , mapperFunction, thisArg ] )
• 22.2.4.2.1 AllocateTypedArray ( constructorName, newTarget, defaultProto [ , length ] )
• 24.1.1.3 DetachArrayBuffer ( arrayBuffer [ , key ] )
• 24.1.1.10 GetValueFromBuffer ( arrayBuffer, byteIndex, type, isTypedArray, order [ , isLittleEndian ] )
• 24.1.1.12 SetValueInBuffer ( arrayBuffer, byteIndex, type, value, isTypedArray, order [ , isLittleEndian ] )
• 24.1.1.13 GetModifySetValueInBuffer ( arrayBuffer, byteIndex, type, value, op [ , isLittleEndian ] )
• 24.4.1.1 ValidateSharedIntegerTypedArray ( typedArray [ , waitable ] )
• 25.6.5.4.1 PerformPromiseThen ( promise, onFulfilled, onRejected [ , resultCapability ] )

(There's also the direction parameter for some definitions of regex evaluation, which is an odd case.)

[devsnek]

Some of these optional parameters help contain the invariants of an operation. For example GetExportedNames and ResolveExport required passing in an empty list at the top-level call. The recursive nature of those operations is not really of any concern to the caller, so making the parameter optional helps keep details encapsulated.

[jmdyck]

Good point. Though I think those two are the only ops for which that argument applies.

[michaelficarra]

I'm in favour of refactoring out the optional parameters wherever it is reasonable to do so.