chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.79k stars 420 forks source link

continue in FORALL loop generates (incorrect) error message #7626

Closed buddha314 closed 6 years ago

buddha314 commented 7 years ago

Based on this SO Post and Bruce's reply.

var ids = {1,2,3,5,7,11};

forall id in ids {
  if id == 5 then writeln("High Five!!");
  if id == 7 then continue;
  writeln(id);
}

Yields

thing.chpl:5 error: break or continue is not in a loop
bradcray commented 7 years ago

I didn't want to muse about this on the SO issue, but I'm trying to remember whether it was our intention to support continues (but not breaks) within forall loops. My sense is "yes", but I can't recall for sure. The challenge with breaks is that it would logically imply shooting down all the other concurrent tasks that are running, which is a significant challenge (at least, to do efficiently). I thought we'd determined that continue statements could be implemented simply by continueing within the serial loop that a given task encountering the statement is running? Does anyone remember better than I do?

bradcray commented 7 years ago

Ah yes, here's what I was thinking of, suggesting we had decided it should be legal, but may not have ever implemented it: https://github.com/chapel-lang/chapel/commits/master/test/statements/sungeun/continue_forall_label.chpl

@daviditen, @vasslitvinov, do either of you recall anything about this?

lydia-duncan commented 7 years ago

It might also be that the "continue" statement works in an if/else with curly braces but not in the one-line syntax. But I also don't have a lot of trust in our testing of variations like this.

bradcray commented 7 years ago

It might also be that the "continue" statement works in an if/else with curly braces but not in the one-line syntax.

Rewriting the test in that way shows that this is not true—the behavior is the same.

bradcray commented 7 years ago

https://www.youtube.com/watch?v=EW1Frr4OcRc

daviditen commented 7 years ago

I really thought that we had continue working in forall loops at one point, but going as far back as 1.9 that seems to not be the case. In 1.15 and earlier we at least had a better error message:

contin.chpl:5: error: continue is not allowed in forall statement

buddha314 commented 7 years ago

So, I know this thread is about the error message, is there also one about (a) how it hurts my feelings and (b) how we are going to jump out of forall loops?

vasslitvinov commented 7 years ago

I suspect the error message is easy to fix.

bradcray commented 7 years ago

is there also one about (a) how it hurts my feelings

You should know by now we don't care about your feelings, Bruce.

and (b) how we are going to jump out of forall loops?

I think this is now that issue (which is why I put "incorrect" in parens).

That said, note that continue doesn't jump out of loops, it just skips ahead to the next iteration. break jumps out of loops but is not planned for support in forall loops (at least in the forseeable future) in Chapel due to the implementation challenges mentioned above.

If what you want is continue, then I think we should just make that happen on this issue. If you also want to be able to use break in a forall loop, then I think that should be a new issue, though I think it's going to be a much more controversial one.

Vass writes:

I suspect the error message is easy to fix.

It'd be great to have the error message not be an outright lie as it is now if it's 5-15 minutes of work. But I think getting continue to work in foralls would be the satisfactory way to close this issue.

buddha314 commented 7 years ago

The real request is to have it "continue", as in "stop on this particular one", not "stop on the whole loop".

vasslitvinov commented 6 years ago

Resolved by #8125 .

Now, inside a forall: continue works as in a for-loop, break results in a reasonable error message.