continue and break should not be allowed on RHS of expression

[adienes]

for ix in 1:10
    x = continue
    println(ix)
end

this continues through the whole loop, but x remains undefined. I would expect a syntax error here. the behavior is similarly confusing in cases like this

for i in 1:10
    for j in continue end
    println(i)
end

[martinholters]

Then probably return, throw and @goto, too. Anything else?

[Seelengrab]

The title should probably read "assignment", not "expression"; those don't have a RHS, they're a block of expressions/literals. I'm using the (non-performance critical) return isempty(failures) || return failures in my code to either return true or the failures for a test function - very useful to then call @test myfunc() and get the failures, if any, recorded in a TestSet .

[martinholters]

return isempty(failures) || return failures

Isn't that identical to return isempty(failures) || failures ?

But upon seeing that, I vaguely remember there might be an issue already about using return-style expressions in positions expecting a value, not just on the RHS of assignments.

[Seelengrab]

Yes, seeing non-booleans as arguments to control flow feels very odd though :) If we disallow that, I don't think the common pattern

check_args(args...) || throw(ArgumentError(...))

will continue to work either.

[martinholters]

Isn't throw(ArgumentError(...)) also non-boolean? Doesn't feel that odd...
So yes, for || and &&, the second argument is not expected to provide a value, it can also reasonably be a jump somewhere else. E.g. isempty(arg) && return 0 also is perfectly fine.

BTW, I'm not at all convinced we should disallow any of this in the parser. Might better be a job for a linter.

[Seelengrab]

Isn't throw(ArgumentError(...)) also non-boolean? Doesn't feel that odd...

Yes, but throw is at least control flow, which just failures wouldn't be!

BTW, I'm not at all convinced we should disallow any of this in the parser. Might better be a job for a linter.

Yes, agreed.

[adienes]

the short circuiting operators kind of own their own terrain; I agree that constructions like cond && break are convenient and cannot be removed, even if they can allow for occasional bad style that should be caught by a linter

what seems much more controversial to me, and what I believe should be a syntax error, is allowing return, break, continue in "non-terminal" expressions (unsure of the correct technical term). that is, the control flow will trigger even when only evaluated as part of an argument list or as a component of some other expression. observe:

### prints nothing
for ix in 1:10
    +(1, continue)
    println(ix)
end

### ERROR: syntax: unexpected "," after continue
for ix in 1:10
    +(continue, 1)
    println(ix)
end

and of course the original example with x = continue also seems to me more like a bug than linting problem

in fact, I bet the following two rules would catch 99% of cases:

1. break, continue, return must be in the same hard local scope as surrounding for, while, return
2. break, continue, return cannot appear in function arguments or assignment statements

since || and && neither make a new scope nor are functions, all usage of control flow with the short-circuiting operators is preserved

throw is just a function, so I don't think it needs any special casing. @goto is... well, I think if you're using @goto much you should be prepared for strange behavior anyway---plus, macros get to break the rules 🙂

[Seelengrab]

FYI, disallowing continue/break on the RHS of an assignment would disallow patterns like

for x in vec
    y = @something discombobulate(x) continue
    println(2*y)
end

because @something foo(x) continue expands to

julia> @macroexpand @something foo(x) continue
quote
    var"#3###233" = foo(x)
    if !(Base.isnothing(var"#3###233"))
        Base.something(var"#3###233")
    else
        begin
            var"#4###232" = continue
            if !(Base.isnothing(var"#4###232"))
                Base.something(var"#4###232")
            else
                Base.something(Base.nothing)
            end
        end
    end
end

(Note the var"#4###232" = continue).

Maybe it's worth changing @something for this, the question is just if there are other things in the ecosystem relying on this implicitly.

[jariji]

Afaik the compiler normally allows unreachable code without complaint. So this feels more like a linter thing to me.

[adienes]

the x = is not unreachable, it's just ignored after the continue is found

I don't think this is "just a linter" issue. more broadly it's pretty inconsistent when continue / break are or are not legal in expressions and a lot of the time when they are it makes no sense. e.g. why is

julia> for i in 1:10
           foo(x::T) where T <: continue
           println(i)
       end

legal but not when I wrap the where in braces, so this gives syntax error

julia> for i in 1:10
           foo(x::T) where {T <: continue}
           println(i)
       end
ERROR: ParseError:
# Error @ REPL[8]:2:35
for i in 1:10
    foo(x::T) where {T <: continue}
#                                 └ ── unexpected token after continue