Remove unnecessary ReturnIfAbrupt calls

[anba]

Fixes
https://bugs.ecmascript.org/show_bug.cgi?id=4445
https://bugs.ecmascript.org/show_bug.cgi?id=4460
https://bugs.ecmascript.org/show_bug.cgi?id=4461
https://bugs.ecmascript.org/show_bug.cgi?id=4475

The ReturnIfAbrupt after GetBindingValue is the only where an explicit assertion makes sense.

[bterlson]

I'm not a huge fan of removing the 3 of these that aren't replaced with an assert. Yes subsequent abstract ops handle abrupt completion input values, but it makes it a bit easier to understand that, for example, the subsequent call to Call does nothing when GetMethod fails.

That said, I'm ok taking this. I plan to solve the explicit ReturnIfAbrupt requirement soon. I want to do this by updating the notational conventions to state that calls to abstract operations are implicitly followed by a ReturnIfAbrupt (and therefore return values) unless prefixed by a ?, in which case you get a completion value out instead. Eg.

1. Let _result_ be Call(_method_, _obj_).

is equivalent to

1. Let _result_ be ?Call(_method_, _obj_).
1. ReturnIfAbrupt(_result_).

But I've yet to convince myself that this will work for the algorithms that explicitly defer handling of abrupt completions.

[bterlson]

Committed as f19635e.

[rossberg]

@bterlson, if you want to shorten the noisy ReturnIfAbrupts then I strongly suggest going the other way round: introduce an abbreviating operator for wrapping ReturnIfAbrupt around a call. Say,

1. Let _result_ be ! Call(_method_, _obj).

is short for

1. Let _result_ be Call(_method_, _obj_).
1. ReturnIfAbrupt(_result_).

This is much clearer. It was a major improvement of the ES6 spec over the ES5 spec to make the places of possible failure explicit. Please don't reverse that!

[bterlson]

@rossberg-chromium What you're arguing for is that almost every call to an abstract operation be prefixed by !. To me this is less clear because it's easy to miss the exceptions. I'd rather the exception be notated differently because it seems easier to understand and easier on authors to specify the correct behavior.

Also this is not a reversal in the sense that we would go back to ES5 implicitness - ES2016 would explicitly state that all calls to abstract operations are wrapped in ReturnIfAbrupt unless the author explicitly says they don't want that behavior by prefixing with ?. This seems fine to me. Thoughts?

[bterlson]

@anba pointed out that the bad part about my strawman is that it makes it impossible to assert that the result of the call wasn't an abrupt completion and I do find this to be a very useful explicit signal when implementing. Therefore, I amend my strawman to:

1. Let _result_ be Call(_method_, _obj_).
1. Let _result2_ be !Call(_method_, _obj_).
1. Let _completionResult_ be ?Call(_method_, _obj_).

As shorthand for today's

1. Let _result_ be Call(_method_, _obj_).
1. ReturnIfAbrupt(_result_).
1. Let _result2_ be !Call(_method_, _obj_).
1. Assert: _result2_ is never an abrupt completion.
1. Let _completionResult_ be Call(_method_, _obj_).

@anba still doesn't much like this proposal so if I'm the odd man out we can go with requiring calls to be prefixed by ? unless you want a completion value..

[rossberg]

FWIW, the latter makes ? the bind operator for the completion monad. That is, it makes perfect sense. :)

[bterlson]

Does latter mean my proposal? If so, then yes I agree, and that was totally my intention. ;)

[rossberg]

"Latter" referred to the last sentence in your previous comment (which seems to be the same as what I proposed before but writing it ?).

[bterlson]

Sorry @rossberg-chromium , I was meaning to say 'prefixed by !`, in other words exactly your proposal. To clarify, we have the following strawmen:

Brian

1. Let _result_ be Call(_method_, _obj_).
1. Let _result2_ be !Call(_method_, _obj_).
1. Let _completionResult_ be ?Call(_method_, _obj_).

Andreas

1. Let _result_ be ! Call(_method_, _obj_).
1. Let _result2_ be Call(_method_, _obj_).
1. Assert: _result2_ is never an abrupt completion.
1. _result2_ = _result2_.[[value]].
1. Let _completionResult_ be Call(_method_, _obj_).

Andreas.1

If you agree that ! makes more sense as a "do this and never fail" operator and ? as a "this might fail, return if it does" operator, you could go with:

1. Let _result_ be ?Call(_method_, _obj_).
1. Let _result2_ be !Call(_method_, _obj_).
1. Let _completionResult_ be Call(_method_, _obj_).

[domenic]

+1 to the "Brian" variant. I both write and read a lot of ES-style specs, and can say with certainty that would improve both sides of the experience. The best part of it, IMO, is it gives a better mapping between ES code and Ecmaspeak: in both ES and in Brian-dialect Ecmaspeak, simply performing an operation will automatically return-if-abrupt. The additional ? and ! shorthands are perfect lightweight conveniences for the uncommon-but-important cases that they map to, which you layer on to the base behavior.

[allenwb]

A agree with @rossberg-chromium's initial reply

It was a major improvement of the ES6 spec over the ES5 spec to make the places of possible failure explicit. Please don't reverse that!

Prior to ES6 the onvention was that exceptions implicitly propagated, with the result that most often neither spec. writers nor ES implementators ever thought about them. This resulted in significant variation among implementations in the actual observable points where an particular exception might occur. Usually this wasn't very significant, but with the introduction of proxies, accessors, constructor super calls, and other new forms of call-backs into ES code there are many more ways that the interweaving of side-effects and exceptions can be observed.

I think it is important for the spec. conventions to nudge that spec. writers and readers to be always paying attention to where an abrupt completion could actually occur. Making ReturnIfAbrupt the default call behavior would be a step backwards in that regard.

Here is what suggest:

1. Let result be !Call(method, obj)
2. Assert: result is result of applying ReturnIfAbrupt to completion of Call
3. Let result2 be ?Call(method, obj).
4. Assert: result is the completion record returned from Call
5. Asset: the following call never returns an abrupt completion
6. Let result3 be Call(method, obj).
7. Assert: result3 ihas an implicit: Let result3 = result3.[[value]]

[jmdyck]

So both 1 and 6 have an implied call to ReturnIfAbrupt??

[allenwb]

@jmdych
1 does, but 6 doesn't need one. Instead it has an implicit: Let result3 = result3.[[value]]

updating my suggestion to make that clearer

[bterlson]

So while it is clearly the right choice from a usability perspective to default to the most common calling convention (call followed by RIA, ie the "Brian" strawman), I can see @allenwb/@rossberg-chromium's point that authors should be pushed to think about it more by making it explicit and that implementers should not be assuming anything on their own.

I like @allenwb's proposal, personally (prefer "?" to mean give me a completion record, as it's kind of saying to me "I don't know how this will complete, so give me back a record I can inspect later). My only concern is that it doesn't address the verbosity of "Assert: result is never an abrupt completion". I think if we adopted that proposal we would leave all the current asserts in as they are a very useful deliberate signal to implementers. But this is still such a drastic improvement that I'm going to work on implementing this now and we can think about simplifying further later.

[rossberg]

I actually like your "Andreas.1" proposal best.

[bterlson]

Having the default semantics be "you get a completion record" also makes the conversion from current spec text to new spec text less error prone because not updating a call keeps its current semantics in place, whereas with Allen's proposal, value extraction is added.

Additionally, asserting that the result of an abstract operation was never an abrupt completion is actually pretty rare - it occurs only 40 times in the spec, compared to ~935 times for the simple RIA case.

I've got a really bad case of semantic satiation with all these ?'s and !'s so I can't really argue effectively for any. So I'm going to make a PR to start with saying "?" means RIA and we can go from there later if we want. For example we might end up wanting to make "!" mean "extract completion's [[value]]" (henceforth, the Andreas-Monadic Operator), but let's get experience with the ? operator before adding that.

[rossberg]

Actually, the one doing the implicit RIA would be the monadic bind. ;) I'm fine with using `?` for this, and I agree that the implicit Assert is less important, and we perhaps shouldn't overdo it. So SGTM.