Editorial: Set [[SourceText]] inside OrdinaryFunctionCreate

[jmdyck]

Every call to OrdinaryFunctionCreate is closely followed by a step that sets the [[SourceText]] of the resulting function object. Replace all but one of the latter steps by passing another argument
to OrdinaryFunctionCreate.

---

Re "all but one": the exception is in CreateDynamicFunction, where there isn't a Parse Node that represents the whole function, so we just pass ~empty~ to OrdinaryFunctionCreate and set the function's [[SourceText]] explicitly later. If you'd like this case to be handled inside OrdinaryFunctionCreate as well, I can think of a few ways:

• Say that the new parameter is either a Parse Node (handled as in this PR) or it's a sequence of Unicode code points (which gets assigned directly to _F_.[[SourceText]]). In CreateDynamicFunction, we'd move up the definitions of _prefix_ and _sourceString_, and then pass UTF16DecodeString(_sourceString_) to the new parameter.

• Say that the new parameter is always a sequence of Unicode code points. CreateDynamicFunction would be handled as above, but all the other invocations would be preceded by a step Let _sourceText_ be the source text matched by |Foo|. and then pass _sourceText_ rather than |Foo| to the new parameter. This would undo most of the savings of this PR, although we could regain most of it if we defined an abstract operation that returned the source text matched by a Parse Node; then we could avoid the extra step and pass (say) SourceText(|Foo|) to the new parameter.

• Say that the new parameter is always a Parse Node. In CreateDynamicFunction, you'd have to actually parse UTF16DecodeString(_sourceString_) and then pass the result to the new parameter. Note that we're already parsing most of it, to get the values of _parameters_ and _body_, so parsing the whole thing isn't that big a stretch (and you could extract _parameters_ and _body_ from it, instead of parsing them separately). The goal symbol for the parse would depend on _kind_, so we could specify that in a new column in the following table.

---

Downstream uses:

• Web IDL: none
• HTML: One invocation. With the PR as given, it could just pass ~empty~. With any of the bulleted alternatives, it would have to do something else (possibly similar to what CreateDynamicFunction does). But it raises the question of what is web reality for the toString of a function created in this way.

[ljharb]

it seems like this should be added last as an optional parameter, then it can default to ~empty~?

[michaelficarra]

#1824 (comment)

The parameter is almost always provided. I don't think using an optional parameter is worth it.

[ljharb]

Then, optional or not, why not at the end?

[jmdyck]

The current parameter list has two Parse Node parameters (_ParameterList_ and _Body_) for syntactic components of the function declaration. Since the new parameter is a Parse Node representing the whole declaration, it makes sense to put it next to those existing Parse Node parameters. Putting it before or after is somewhat debatable, but I put the new one before the existing ones because then the order of nonterminals in the invocations of OrdinaryFunctionCreate is the same as in the associated production. E.g. for production:

FunctionDeclaration : `function` `(` FormalParameters `)` `{` FunctionBody `}`

the invocation is:

OrdinaryFunctionCreate(..., |FunctionDeclaration|, |FormalParameters|, |FunctionBody|, ...)

I think this makes it easier to understand/remember the order of parameters for this operation.

[michaelficarra]

@jmdyck We discussed this during the editor call today. We'd feel more comfortable if the use of "the source text matched by" remained in the algorithm steps of the syntax-directed operations. We could still consolidate the assignment to [[SourceText]] in OrdinaryFunctionCreate by passing in the result of "the source text matched by" instead if you'd like to update your PR in that way. After that, we can do another review.

[jmdyck]

@jmdyck We discussed this during the editor call today. We'd feel more comfortable if the use of "the source text matched by" remained in the algorithm steps of the syntax-directed operations.

I'm willing to go along with this, but (for possible future reference), can you articulate why it makes you feel more comfortable?

We could still consolidate the assignment to [[SourceText]] in OrdinaryFunctionCreate by passing in the result of "the source text matched by" instead if you'd like to update your PR in that way.

To be clear, are you saying

1. Let _sourceText_ be the source text matched by |Foo|.
1. ... OrdinaryFunctionCreate(..., _sourceText_, ...).

or

1. ... OrdinaryFunctionCreate(..., the source text matched by |Foo|, ...).

?

Re the latter, it's uncommon for the spec to use a prose-y phrase as an argument to an operation, but it does occur, e.g.:

• ThrowCompletion(a newly created *TypeError* object)
• Type(_V_'s base value component)
• NewDeclarativeEnvironment(_runningContext_'s LexicalEnvironment)
• ForBodyEvaluation(the first |Expression|, the second |Expression|, ...)

Or, to avoid the prose-y argument, we could 'extract' the source text via an operation. I suggested SourceText(|Foo|), but it could be TheSourceTextMatchedBy(|Foo|) if you really want that phrase to appear in the algorithm that calls OrdinaryFunctionCreate.

[ljharb]

Either, but I'd prefer the former.

[michaelficarra]

@jmdyck We feel more comfortable about it because the referenced nonterminal is lexically present in the syntax-directed operation, whereas it is not present in the abstract operation. It is more clear when the nonterminal is present.

Like @ljharb, I would prefer the former.

[jmdyck]

(force-pushed a version that should be more acceptable to the editors)

[jmdyck]

(force-pushed to resolve merge conflicts)

[jmdyck]

(force-pushed to resolve merge conflicts)

[syg]

lgtm. Holy cow there are a lot of users of OrdinaryFunctionCreate.