Skip to content

break/continue make all list comprehensions [] #1669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TrevorBurnham opened this issue Sep 5, 2011 · 37 comments
Closed

break/continue make all list comprehensions [] #1669

TrevorBurnham opened this issue Sep 5, 2011 · 37 comments
Labels

Comments

@TrevorBurnham
Copy link
Collaborator

The code

func = (x, y) ->
   for x in y
     x
     break if condition

compiles as if it were a list comprehension—except that _results is never modified:

var func;
func = function(x, y) {
  var _i, _len, _results;
  _results = [];
  for (_i = 0, _len = y.length; _i < _len; _i++) {
    x = y[_i];
    x;
    if (condition) break;
  }
  return _results;
};

As a result, func will always return []. Same thing if you substitute continue instead of break.

If you make it return if condition, you get no signs of a list comprehension whatsoever, which I find a little disappointing, but at least internally consistent.

@satyr
Copy link
Collaborator

satyr commented Sep 5, 2011

related: satyr/coco#79

@boundvariable
Copy link
Contributor

If you break from a for, while, loop or until then what was the last expression in the function?

Ruby has implicit returns... and a break within a block acts like a return and will have the outer method, proc or lambda return nil unless you pass it a value - something similar might be another option.

CS generally returns an empty array instead:

a = ->
  while false then true

a()
# []

b = ->
  while true
    break

b()
# []

So it seems fairly consistent.

That said, I didn't expect this:

d = ->
  for x in [1,2,3]
    x
    break if false

d()
# []

@jashkenas
Copy link
Owner

I think that last example is actually expected.

d = ->
  for x in [1,2,3]
    x
    break if false

... I'm afraid that there's no bones about it -- x is not the last expression in that loop body, and should not be considered the result of the iteration. Naturally, you'd write it like this:

d = ->
  for x in [1,2,3]
    break if false
    x

@TrevorBurnham
Copy link
Collaborator Author

I guess you could call it consistent, but not very useful. Wouldn't the rule "implicit returns give you the value of the last non-control expression" be more useful, since break/continue never have any value? I'd want

d = ->
  for x in [1,2,3]
    x
    break

to give me [1], not [].

At any rate, surely it's inconsistent that

d = ->
  for x in [1,2,3]
    break if false
    x

gives you [1,2,3], but

d = ->
  for x in [1,2,3]
    if false then break else x

gives you []?

@jashkenas
Copy link
Owner

Yep.

@TrevorBurnham
Copy link
Collaborator Author

So it sounds like we're agreed that Coco's approach would be an improvement here. Want to make $10, @satyr? https://github.com/jashkenas/coffee-script/wiki/Bug-Bounties

@satyr
Copy link
Collaborator

satyr commented Sep 6, 2011

we're agreed that Coco's approach would be an improvement here

Are we saying that with full understanding of my approach? The notable difference is:

$ coffee -e 'console.log(x+y for y in [1,2] for x in [3,4])'
[ [ 4, 5 ], [ 5, 6 ] ]

$ coco -e '(x+y for y of [1 2] for x of [3 4])'
[ 4, 5, 5, 6 ]

@TrevorBurnham
Copy link
Collaborator Author

Right, no, we don't want that. I was referring to the example in the issue you linked to:

console.log(
  for n in [2..-2]
    if n >= 0 then n else break
)

ought to show [2, 1, 0], not []. Jeremy agreed that was inconsistent, so the bounty is for fixing that.

@satyr
Copy link
Collaborator

satyr commented Sep 6, 2011

ought to return [2, 1, 0]

Which requires recursive application of _results.push (in the same way return works) which leads to Coco's behavior.

@TrevorBurnham
Copy link
Collaborator Author

Surely there must be some other way to get the loop to compile to

for (n = 2; n >= -2; n--) {
  if (n >= 0) {
    _results.push(n);
  } else {
    break;
  }
}

rather than the current

for (n = 2; n >= -2; n--) {
  if (n >= 0) {
    n;
  } else {
    break;
  }
}

?

@satyr
Copy link
Collaborator

satyr commented Sep 6, 2011

Making for/while an exception to the traversal is surely possible, but seems rather inelegant.

Are you grokking how exactly the current behavior is achieved?

@TrevorBurnham
Copy link
Collaborator Author

No, I prefer to stay away from implementation details, and just look at behavior. I get that, in a list comprehension,

if x then y else z

compiles to

_results.push(x ? y : z);

and more complex expressions like

try
  ...

compile to

_results.push((function() { ... })());

So conditional control expressions like if x then y else break have to compile quite differently. And if you want to get into some really messy examples...

    try
      break if x
      y

That won't work with the way try currently compiles to a function wrapper in a list comprehension. It should really compile to

try {
  if (x) break;
  _results.push(y);
}

So, I get that this would be a fairly major implementation change. But if someone figured out a way to solve it, while preserving the current behavior of nested list comprehensions, the CoffeeScripters of the world would thank them.

@satyr
Copy link
Collaborator

satyr commented Sep 6, 2011

So you want:

while a
  b while c

to become

while a
  __results.push(b while c)

but:

while a
  b if c

to become

while a
  __results.push(b) if c

Correct?

@TrevorBurnham
Copy link
Collaborator Author

No, the current behavior of

while a
  b if c

should remain the same. But

while a
  if c then b else break

should become

while a
  if c then __results.push(b) else break

@satyr
Copy link
Collaborator

satyr commented Sep 6, 2011

That doesn't make sense to me. Can you explain the algorithm in detail?

@TrevorBurnham
Copy link
Collaborator Author

if c then b (implicitly if c then b else undefined) is an expression with a value. That value should always be appended to __results.

if c then b else break, on the other hand, is not a normal expression—it has a value if c, but no value otherwise. Either c should be appended to __results, or the loop should end and __results remain unchanged.

Rather than having potentially several __results.push expressions in a complex list comprehension, perhaps it would make sense to have a __result variable that gets pushed to __results at the very end of the loop, ensuring that nothing would be appended if a break or continue occurred. For instance,

while a
  try
    if x then y else break

might compile to

while (a) {
  try {
    if (x) {
      __result = y;
    } else {
      break;
    }
  } catch (_error) {}
  __results.push(__result);
}

@satyr
Copy link
Collaborator

satyr commented Sep 6, 2011

if c then b else break, on the other hand, is not a normal expression

The algorithm in question is for determining that normal-ness. Which of below are normal?

  • if a then b
  • if a then b else c
  • if a then b else break
  • if a then b else continue
  • if a then b else return
  • switch ...

And what about loops that contain above?

  • while a then if b then c else break
  • ...

@TrevorBurnham
Copy link
Collaborator Author

Which of below are normal?

The first two are normal. Any conditional (be it if or switch) is not. Anything involving return is irrelevant because return is disallowed in list comprehensions.

Loops that contain their own breaks and continues are normal expressions, since they'll always have a value (specifically, an array).

@satyr
Copy link
Collaborator

satyr commented Sep 6, 2011

The first two are normal. Any conditional (be it if or switch) is not.

Can you clarify--the first two also seem conditional to me.

@TrevorBurnham
Copy link
Collaborator Author

Typo—I meant "any conditional involving break or continue."

@satyr
Copy link
Collaborator

satyr commented Sep 6, 2011

any conditional involving break or continue

"... as the last expression of one of its branches," right? Consider:

if b
  break if d
  c

@satyr
Copy link
Collaborator

satyr commented Sep 6, 2011

I guess I'm ready to hunt, as long as @jashkenas sees the above instruction reasonable.

@jashkenas
Copy link
Owner

Yep, Trevor's laying out of the rules sounds just about perfect.

@TrevorBurnham
Copy link
Collaborator Author

"... as the last expression of one of its branches," right?

Right, so let's do a more complex example to get to the heart of this:

for x in y
  a
  if b
    break if d
    c

Clearly, if b and not d, this needs to push c to __results. But what if:

  • b and d?

Does a get pushed to __results, or nothing?

  • not b?

Same question.


I'd argue that in both cases, the ideal behavior is for a to get pushed to __results. But, that would 1) increase the complexity of compiled output and 2) intuitively imply that the current behavior of

a
if b
  c

would have to change, so that either a or c could be returned (rather than c or undefined). And that would apply to functions, not just loops...

So let's say "no, a can't be returned" for now. As long as the answer to both of the above questions is the same, we have consistency, and that's the most important thing.

@jashkenas
Copy link
Owner

a is not a part of the final expression in the loop body, and should never get pushed to the results, regardless of the values of b or d.

@TrevorBurnham
Copy link
Collaborator Author

Right, I was just talking about my intuition. Even Ruby doesn't go that far, though:

def func
  1
  2 if false
end

puts func  # nil

@satyr
Copy link
Collaborator

satyr commented Sep 7, 2011

Eh, so what, still undecided? Would

if b
  break if d
  c

become

if b
  break if d
  _results.push c

or

if b
  break if d
  _results.push c
else
  _results.push undefined

?

@jashkenas
Copy link
Owner

I think:

if b
  break if d
  _results.push c

... with the idea that break is intending to skip the current iteration of the loop.

@TrevorBurnham
Copy link
Collaborator Author

Exactly. If a break or continue is hit, there should be no __results.push in that loop iteration.

@satyr
Copy link
Collaborator

satyr commented Sep 7, 2011

So whether to push undefined for else-less if is determined by what exactly, the existence of break/continue earlier within the same block? Tricky.

@jashkenas
Copy link
Owner

I'm sorry -- I misunderstood, and didn't read closely enough. (under deadline here -- really can't pitch in much tonight).

if b
  break if d
  _results.push c
else
  _results.push undefined

... is correct.

@satyr satyr closed this as completed in ad1bc1e Sep 7, 2011
@satyr
Copy link
Collaborator

satyr commented Sep 7, 2011

Uh, auto-close misfired.

Anyway, does that look ok?

@jashkenas
Copy link
Owner

Looks great -- nicely done.

@showell
Copy link

showell commented Sep 7, 2011

Has the whole notion of _results being returned at all been put up to serious debate before? It seems to open up a can of worms, and it's a particularly hard thing to sell to CS skeptics, mainly in terms of readability of the generated JS code. Do people take advantage of this feature in real production code? Why does CS return an array where Ruby returns the loop var (and Python/JS return nothing)?

@TrevorBurnham
Copy link
Collaborator Author

Has the whole notion of _results being returned at all been put up to serious debate before?

Yes, most notably at #896. That also spawned a secondary discussion of an alternate syntax (e.g. -/>) for defining functions with no return values at #899.

@TrevorBurnham
Copy link
Collaborator Author

As to the commit—well done, satyr! Bounty closed.

@satyr
Copy link
Collaborator

satyr commented Sep 7, 2011

Bounty closed

Keep'em coming. =D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants