Editorial: update use of, or stop using, completion values?

[domenic]

tc39/ecma262#2547 has changed how completion records work. Now, if an operation is infallible, the recommended practice is have it not return a completion record. Also, everything has a header saying what it returns.

We could update to follow, similar to whatwg/html#7661 .

Or, we could try to further harmonize with the rest of the web spec ecosystem, and stop doing completion records entirely. (Except where necessary to interface with ECMAScript.) I think I'd kind of prefer that. Concretely, we'd:

• Move away from ECMAScript abstract ops as much as possible, e.g. using the stuff introduced in Add and improve operations on BufferSources webidl#987
• Get rid of all ? and !s for internal abstract op calls
• Change all explicit completion processing, e.g. instead of "If result is an abrupt completion, return a promise rejected with result.[[Value]].", say "If this throws an exception, catch it and return a promise rejected with that exception."
• Update any remaining use of ECMAScript abstract ops to appropriately use or not use ?/!

[ricea]

I like the ? and ! but there's no doubt they're super confusing to people coming to the spec for the first time.

Would we have to explicitly write "Assert: This does not throw an exception" everywhere?

[domenic]

Would we have to explicitly write "Assert: This does not throw an exception" everywhere?

We wouldn't have to. We could if it's helpful. I'd suggest only doing it in cases where the not-throwing is potentially surprising. (Which is of course subjective.)

This is related to the question of whether/how we should introduce headers to the abstract operations, like ECMAScript now has.

If we keep !/? and just follow ECMAScript, then the headers could look something like:

AcquireWritableStreamDefaultWriter(stream) returns either a normal completion containing a WritableStreamDefaultWriter or an abrupt completion. It performs the following steps:

IsReadableStreamLocked(stream) returns a boolean. It performs the following steps:

ReadableStreamAddReadRequest(stream, readRequest) returns unused. It performs the following steps:

Or they could be omitted if we think they're not worthwhile.

Whereas if we do web spec style, then I think omitting is probably more conventional, since most specs are not very explicit about whether their operations can throw or not; it's just kind of assumed things will flow throughout. But if we did want headers they would look something like:

AcquireWritableStreamDefaultWriter(stream) either returns a a WritableStreamDefaultWriter or throws. It performs the following steps:

IsReadableStreamLocked(stream) returns a boolean. It performs the following steps:

ReadableStreamAddReadRequest(stream, readRequest) performs the following steps:

[ricea]

For Blink's implementation, knowing whether something can throw is useful, because it changes the calling convention.

However, in practice there's stuff that can throw in Blink that never throws in the spec, so we can't use the spec as the ultimate source of truth. For example, stack overflow and Worker termination can cause many V8 operations to throw.

[mgaudet]

Gecko has the same story: Calling convention changes for throwing algorithms, and places where we will throw that the specification says has throwing impossible.

Minimizing these cases is on my mind at the moment, and so I've been using completion records in the specification to help figure out what's expected to throw and what's not expected.

In my opinion they're valuable information and I'd be sad to see them go.

[domenic]

Thanks, this is valuable feedback. Would something like my "But if we did want headers they would look something like:" from the OP work, or is seeing the !/? at call sites especially valuable?

[mgaudet]

I guess one thing I don't quite understand is the structure of the proposed headers. You're suggesting something like

SetUpWritableStreamDefaultController(stream, controller, startAlgorithm, writeAlgorithm, closeAlgorithm, abortAlgorithm, highWaterMark, sizeAlgorithm) performs the following steps, returning a completion value, which may be an abrupt completion

?

[domenic]

In the last part of #1224 (comment), where I am talking about avoiding completion values but keeping some of the same information, I'm specifically suggesting:

SetUpWritableStreamDefaultController(stream, controller, startAlgorithm, writeAlgorithm, closeAlgorithm, abortAlgorithm, highWaterMark, sizeAlgorithm) performs the following steps, which can throw an exception:

[ricea]

What happens when we call an operation that could throw, but which is annotated with a "!" to indicate that it doesn't throw in this case, is that we turn it into an assert in the implementation.

So one option would be to replace the "!" with asserts, but only when the operation is defined as "can throw".