Normative: Constrain all host hooks to return either normal or throw completions #2442

syg · 2021-06-24T02:02:32Z

This supersedes #2433.

This PR also makes the prose around requirements for host hook
implementations consistent to the template "An implementation of
HostHook must conform to the following requirements:".

syg · 2021-06-24T02:02:42Z

cc @jugglinmike

spec.html

jugglinmike · 2021-06-24T16:20:32Z

spec.html

@@ -10543,7 +10545,8 @@ <h1>Host Hooks</h1>

      <emu-clause id="sec-host-cleanup-finalization-registry" aoid="HostEnqueueFinalizationRegistryCleanupJob">
        <h1>HostEnqueueFinalizationRegistryCleanupJob ( _finalizationRegistry_ )</h1>
-        <p>The abstract operation HostEnqueueFinalizationRegistryCleanupJob takes argument _finalizationRegistry_ (a FinalizationRegistry). HostEnqueueFinalizationRegistryCleanupJob is an implementation-defined abstract operation that is expected to call CleanupFinalizationRegistry(_finalizationRegistry_) at some point in the future, if possible. The host's responsibility is to make this call at a time which does not interrupt synchronous ECMAScript code execution.</p>
+        <p>The abstract operation HostEnqueueFinalizationRegistryCleanupJob takes argument _finalizationRegistry_ (a FinalizationRegistry). HostEnqueueFinalizationRegistryCleanupJob is an implementation-defined abstract operation that is expected to call CleanupFinalizationRegistry(_finalizationRegistry_) at some point in the future, if possible.</p>
+        <p>An implementation of HostEnqueueFinalizationRegistryCleanupJob must conform to the requirements in <emu-xref href="#sec-jobs"></emu-xref>.</p>


The host's responsibility is to make this call at a time which does not interrupt synchronous ECMAScript code execution.

At first glance, I thought the new requirement on host hooks obviated this statement. On second thought, it doesn't seem like it applies to the behavior of an abstract operation invoked after the host hook has returned. Is there some other justification for removing the statement?

The intention here was the "must not interrupt synchronous execution" is more precisely captured by the requirements in #sec-jobs, specifically the stuff around waiting for the execution context stack to be empty. No job queued by a job-queuing host defined AO in ecma262 is allowed to interrupt synchronous execution.

Hm, a strict reading of the #sec-jobs restriction only applies to Job Abstract Closures, which HostEnqueueFinalizationRegistryCleanupJob doesn't explicitly spell out. I'll spell that out so it's clear that it applies.

spec.html

michaelficarra · 2021-07-08T17:30:26Z

spec.html

-    <p>A <dfn id="host-hook">host hook</dfn> is an abstract operation that is defined in whole or in part by an external source. All host hooks must be listed in Annex <emu-xref href="#sec-host-layering-points"></emu-xref>.</p>
+    <p>A <dfn id="host-hook">host hook</dfn> is an abstract operation that is defined in whole or in part by an external source. All host hooks must be listed in Annex <emu-xref href="#sec-host-layering-points"></emu-xref>. A host hook must conform to at least the following requirements:</p>
+    <ul>
+      <li>It must return either a normal completion or a throw completion.</li>


Although it's currently used twice, I don't think we've defined the term "throw completion". We should probably address those existing uses similarly to my suggestion below:

Suggested change

<li>It must return either a normal completion or a throw completion.</li>

<li>It must return a completion with a [[Type]] value of ~normal~ or ~throw~.</li>

This is the same form used in #2448. Perhaps we want to define a term "callable completion" or something that can be used in all of these cases?

I'd prefer not to unless we start referring to this disjunction in more places. Otherwise it's just another thing to look up when the full definition isn't very long.

I'll define a generic shorthand, like "foo completion means a completion with a [[Type]] value of ~foo~".

spec.html

michaelficarra · 2021-07-09T22:26:30Z

spec.html

@@ -171,7 +171,7 @@ <h1>Hosts and Implementations</h1>
    <p>A <dfn id="host">host</dfn> is an external source that further defines facilities listed in Annex <emu-xref href="#sec-host-layering-points"></emu-xref> but does not further define other implementation-defined or implementation-approximated facilities. In informal use, a host refers to the set of all implementations, such as the set of all web browsers, that interface with this specification in the same way via Annex <emu-xref href="#sec-host-layering-points"></emu-xref>. A host is often an external specification, such as WHATWG HTML (<a href="https://html.spec.whatwg.org/">https://html.spec.whatwg.org/</a>). In other words, facilities that are host-defined are often further defined in external specifications.</p>
    <p>A <dfn id="host-hook">host hook</dfn> is an abstract operation that is defined in whole or in part by an external source. All host hooks must be listed in Annex <emu-xref href="#sec-host-layering-points"></emu-xref>. A host hook must conform to at least the following requirements:</p>
    <ul>
-      <li>It must return either a normal completion or a throw completion.</li>
+      <li>It must return a completion with a [[Type]] value of ~normal~ or ~throw~.</li>


Now that you've defined the shorthands, you don't need to make this change.

I made it to be consistent with #2448 as you pointed out. Any preference on going with this or staying with "normal or throw completion" for both?

"normal completion or throw completion" for both seems best to me. It means your addition of the definitions of those terms will race. Maybe just split it out and we get that in now?

Let's just sequence this one before the other -- meeting's in 2 days and we should get consensus for the other one by then.

ljharb · 2021-07-15T03:52:21Z

@syg this will need a rebase before it's mergeable

syg · 2021-07-21T17:55:27Z

Rebased against structured headers.

…completions (tc39#2442) This PR also makes the prose around requirements for host hook implementations consistent to the template "An implementation of HostHook must conform to the following requirements:".

syg requested a review from a team June 24, 2021 02:02

jugglinmike reviewed Jun 24, 2021

View reviewed changes

spec.html Outdated Show resolved Hide resolved

jugglinmike approved these changes Jun 24, 2021

View reviewed changes

ljharb force-pushed the master branch 3 times, most recently from 3d0c24c to 7a79833 Compare June 29, 2021 02:21

michaelficarra reviewed Jul 8, 2021

View reviewed changes

spec.html Outdated Show resolved Hide resolved

syg force-pushed the restrict-host-hooks-completions branch from 9475b14 to aa2b28c Compare July 9, 2021 19:56

michaelficarra reviewed Jul 9, 2021

View reviewed changes

syg added a commit to syg/ecma262 that referenced this pull request Jul 9, 2021

Match language with tc39#2442

47ff771

bakkot approved these changes Jul 12, 2021

View reviewed changes

ljharb added needs consensus This needs committee consensus before it can be eligible to be merged. normative change Affects behavior required to correctly evaluate some ECMAScript source text labels Jul 12, 2021

michaelficarra approved these changes Jul 12, 2021

View reviewed changes

ljharb added has consensus This has committee consensus. and removed needs consensus This needs committee consensus before it can be eligible to be merged. labels Jul 14, 2021

jridgewell mentioned this pull request Jul 14, 2021

July 2021 babel/proposals#75

Open

michaelficarra added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jul 15, 2021

ljharb requested a review from michaelficarra July 15, 2021 03:52

ljharb removed the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jul 15, 2021

syg force-pushed the restrict-host-hooks-completions branch from 6979282 to f811ffc Compare July 21, 2021 17:54

syg added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jul 21, 2021

michaelficarra approved these changes Jul 21, 2021

View reviewed changes

Normative: Constrain all host hooks to return either normal or throw …

61f7950

…completions (tc39#2442) This PR also makes the prose around requirements for host hook implementations consistent to the template "An implementation of HostHook must conform to the following requirements:".

ljharb force-pushed the restrict-host-hooks-completions branch from f811ffc to 61f7950 Compare July 21, 2021 18:42

ljharb merged commit 61f7950 into tc39:master Jul 21, 2021

syg mentioned this pull request Jul 21, 2021

Normative: restrict abstract op. completion type #2433

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normative: Constrain all host hooks to return either normal or throw completions #2442

Normative: Constrain all host hooks to return either normal or throw completions #2442

syg commented Jun 24, 2021

syg commented Jun 24, 2021

jugglinmike Jun 24, 2021

syg Jun 24, 2021

syg Jun 24, 2021 •

edited

Loading

michaelficarra Jul 8, 2021

syg Jul 8, 2021

syg Jul 8, 2021 •

edited

Loading

michaelficarra Jul 9, 2021

syg Jul 9, 2021

michaelficarra Jul 9, 2021

syg Jul 9, 2021

ljharb commented Jul 15, 2021

syg commented Jul 21, 2021 •

edited

Loading

	<li>It must return either a normal completion or a throw completion.</li>
	<li>It must return a completion with a [[Type]] value of ~normal~ or ~throw~.</li>

Normative: Constrain all host hooks to return either normal or throw completions #2442

Normative: Constrain all host hooks to return either normal or throw completions #2442

Conversation

syg commented Jun 24, 2021

syg commented Jun 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syg Jun 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syg Jul 8, 2021 • 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

ljharb commented Jul 15, 2021

syg commented Jul 21, 2021 • edited Loading

syg Jun 24, 2021 •

edited

Loading

syg Jul 8, 2021 •

edited

Loading

syg commented Jul 21, 2021 •

edited

Loading