The proposal's use of completion values are highly unintuitive with loops

[dead-claudia]

(Partial dupe of #10, but different take/angle.)

It's highly unintuitive that this evaluates to 5, rather than [1, 2, 3, 4, 5]:

let range = do {
    for (let i = 0; i < 5; i++) {
        i + 1;
    }
}

I feel that with loops (e.g. for, while), this should seriously be reconsidered.

[dead-claudia]

Related: #15

[Jamesernator]

I've personally always thought the most intuitive value for the for-of would be the iteration return value. e.g.:

function* gen() {
    if (Math.random() > 0.5) {
        return "Early complete"
    }
    yield 1
    yield 2
    yield 3
    return "Complete"
}

const completion = do { for (const item of gen()) {} }

[ljharb]

That actually makes a ton of sense to me.

Maybe we’d just want to consider banning do/while and for/for..in?

[dead-claudia]

@ljharb Now that I think of it, @Jamesernator's idea is a pretty good idea - having access to the return value of for ... of could be genuinely useful - you currently can't get it outside yield* or direct calls to next. I've found myself a few times wanting that return value, and it's unnecessarily complicated to get if you're consuming generators.

As for the rest (e.g. C-style for), I feel it'd probably make the most sense to just return undefined since it's not always clear whether it should be considered to even have a return value. (It's a similar ambiguity to exponentiation with negative bases - it's unclear whether -x**2 should be (-x) ** 2 or -(x ** 2).)

[ljharb]

Always returning undefined from a non-for-of loop would also make sense to me.

[jorendorff]

Granting this is decidedly unintuitive... hasn't this ship already sailed? The value of a statement is already observable using eval().

1. Breaking compatibility is not an option (unless a browser vendor wants to put in the time to try it and find out what breaks).

2. So then, do we propose making loop evaluation rules different when inside a do-expression? That seems bad to me too; we'd break the refactoring principles in the proposal.

[pitaj]

Breaking eval happens all of the time. Every addition to the language results in a breaking change in eval behavior.

[dead-claudia]

@jorendorff

2. So then, do we propose making loop evaluation rules different when inside a do-expression? That seems bad to me too; we'd break the refactoring principles in the proposal.

That is exactly my proposal, although I left it implicit. I updated the title to clarify this distinction.

[jorendorff]

@pitaj Browser vendors will not implement a new feature that breaks any non-negligible number of existing web sites in practice. Adding new syntax tends not to break web sites. Changing the behavior of an existing feature, like eval, breaks web sites.

[jorendorff]

@isiahmeadows

OK, how about this one:

do {
  for (let item of [1, 2, 3, 4, 5]) {
    if (item > 2) {
      item
    }
  }
}

Should it produce [undefined, undefined, 3, 4, 5] or just [3, 4, 5]?

I think [3, 4, 5] is more intuitive, but it seems to require the language to treat for and if as combining to do something other than what the two parts do separately.

[domenic]

I'm a little skeptical there are websites depending on eval completion values. We changed them fairly drastically in ES2015; did you see any complaints from developers at that time?

[jorendorff]

What changed? I was under the impression the specification text changed a lot, but the resulting behavior was basically the same...

SpiderMonkey doesn't fully comply to the current standard, but I think the noncompliance is limited to places where the spec says the value of a StatementList should be undefined and we instead return the value of an ExpressionStatement somewhere in there. That alone would not give us much wiggle room.

[domenic]

I guess I don't have a comprehensive list, I just thought they were reformed extensively. Paging @allenwb.

[jorendorff]

If we could change it (and I'm skeptical but eager to hear more), what we would want is a design based on preserving common-sense rewrite rules.

I imagine the original design intent was "eval returns the value of the last-evaluated ExpressionStatement", i.e. the selection of a value was a dynamic property of control flow through the code. IIUC this is the ES3 behavior. It's easy to understand and makes sense in a dynamic language. Any rewrite rule that preserves the number and order-of-evaluation of ExpressionStatements holds under this design, trivially.

Another consistent choice might have been: "the value of a StatementList is the value of the last StatementListItem in it, assuming it has any and evaluation completes normally; and undefined otherwise." The selection is syntactic. That's more like Rust. (It's similar to what you'd get if you took the ES3 semantics and got rid of empty everywhere.) Again, reasoning about which rewrite rules are valid here is a snap. Does it affect the last statement of a block?

What we have now is somewhere in between. It's not terrible ...mainly because the differences don't often matter. But where it does differ from ES3, ES3 is easier to reason about and explain to people. And it's hard to say if a rewrite rules holds or not, without perusing the algorithms at length.

[allenwb]

see https://github.com/tc39/proposal-do-expressions/issues/21#issuecomment-359555197

Basically, every statement form that could (perhaps only sometimes) produce a new normal completion value was change so that it ways produced a new completion value (sometimes undefined) and never propagated the previous value.

You can find several issues that came up during development of those changes in http://bugs.ecmascript.org

[allenwb]

The fact that we were able to make changes in ES6 (assuming that at least some implementations actually made those changes) suggests to me that we can probably get away with making other changes.

The one thing I would seriously consider would be giving declarations (or at least function and class declarations) normal completion values instead of an empty normal completion. However, that might complicate doing escape analysis of the values of block scoped declarations..

Otherwise, the rule are conceptually pretty simple:

• All non-declaration statements have a non-empty normal completion value.
• The normal completion value of a Block is the completion value of its StatementList
• The normal completion value of an ExpressionStatement is the value of its expression.
• The normal completion value of all compound statements (statements that embed other statements) is the non-empty normal completion value of the last embedded Statement it evaluated. If it does not evaluate an embedded Statement or the embedded Statement has a normal completion value of empty the compound statement's normal completion value is undefined.
  • The normal completion value of a if statement that lacks an else clause is undefined when the if conditions is falsy.
  • Any IterationStatement that does not evaluate any embedded _Statement_s (ie, performs 0 iteration) has a normal completion value of undefined.
• A break statement produce an abrupt completion with a break completion record whose value is the completion value of the preceding Statement. This may be the empty value.
• The normal completion value of a BreakableStatement that is terminated by a break abrupt completion (either unlabeled or because of a label match) is the value field of the from the abrupt completion record unless that value is empty in which case the completion value is undefined.

[jorendorff]

On IRC, Till Schneidereit suggested considering new syntax break with EXPR; and break LABEL with EXPR; with straightforward semantics:

1. Let r be the result of evaluating EXPR.
2. ReturnIfAbrupt(r).
3. Else r.[[Type]] is normal. Return Completion { [[Type]]: break, [[Value]]: r.[[Value]], [[Label]]: LABEL (or empty if there's no LABEL) }.

Then we could try changing break; to mean break with (void 0); and see what happens. It sure would be great if that didn't break the web.

[bakkot]

The current proposal simply bans loops (and declarations) at the end of do expressions entirely, preventing this from being observable.

[dead-claudia]

@bakkot Should this be closed then?

[bakkot]

@isiahmeadows I'm leaving it open at least until this proposal gets to stage 2 with those semantics. I'm hoping my approach works, but it will require the committee to approve.