Insert explicit return expressions in functions/macros/do-blocks

[fredrikekre]

This patch make sure that function definitions end with an explicit return statement before the last expression in the body.

If for or while is the last expression a return nothing node is inserted after the loop instead of return for ....

if, try, let, and begin are currently are left as they are. Perhaps in the future Runic will recurse down into branches, or insert return if but for now this is left alone.

Another exception is also made whenever the last expression is a call to throw or error because it is pretty clear that the function will not return in this case (return throw(...) look silly).

Closes #43.

[vchuravy]

If for or while is the last expression a return nothing node is inserted after the loop instead of return for ....

Currently broken for @inbounds for ... or @testset "" for

For inserting into macro's like @kernel from KernelAbstractions. Those functions are semantically not allowed to have return statements (yes, I know weird) but they are SPMD programs and one of the assumptions is that instances don't early return.

[fredrikekre]

Currently broken for @inbounds for ... or @testset "" for

Not broken but simply that the last expression is the macrocall here. It does't seem valid to peek inside the macro, but perhaps it can be done for macros that have one argument and the argument is a code block or something? Alternatively, if the last expression is a macrocall it could just be left as is...

For inserting into macro's like @kernel from KernelAbstractions. Those functions are semantically not allowed to have return statements (yes, I know weird) but they are SPMD programs and one of the assumptions is that instances don't early return.

Yea, maybe this (and other existing transformations?) need to be disabled inside macros.

[fredrikekre]

Perhaps it can be done for macros that have one argument and the argument is a code block or something

This would rule out @testset "" for though. It simply isn't possible for Runic to know what comes out of a macro :/

[vchuravy]

I think I like normalizing to return instead of return nothing.

[fredrikekre]

I think I like normalizing to return instead of return nothing.

Right now it will leave return nothing as is but insert return nothing. @KristofferC made a comment that there are two somewhat distinct cases:

1. You want to explicitly indicate you are returning nothing (e.g. findfirst-like functions) and the caller is expected to check === nothing
2. You just want the function to terminate

I think it would be slightly annoying if Runic removed the nothing for 1, and likewise that it added nothing for case 2. I think leaving whats in the source is perhaps the right choice.

Is this meant to be return? Instead of return nothing?

Sure, maybe that is better.

[vchuravy]

Yeah, I just noticed that Runic wasn't consistent in what it inserted. I do agree with not removing explicit "return nothing".

[fredrikekre]

Yeah, I just noticed that Runic wasn't consistent in what it inserted.

Hmm, it should be? If you are thinking about the diff in JuliaGPU/KernelAbstractions.jl#516 the cases where there is just a return added where manual insertions (to avoid e.g. return @testset).

[fredrikekre]

I also noticed in JuliaGPU/KernelAbstractions.jl#516 and JuliaDocs/Documenter.jl#2564 that whenever the function ends with and if it almost always make more sense to add return inside the branches. Thoughts? The question is then if you should also add a else return nothing branch in the cases where no else branch exist? If you reallly want the return if then Runic will leave it as such of course.

[jariji]

Fwiw my preference would be

• A function with only one exit point doesn't need return.
• If one branch returns a value, all branches should return a value.
• A function with multiple exit points in which no branch returns a value just uses return, not return nothing.

[fredrikekre]

A function in which at least one branch returns a value, all branches should return a value explicitly, including return nothing.

Would you then also insert else branches if there is none? E.g.

if cond
    x
end

should become

if cond
    return x
else
    return
end

?

A function in which no branch returns a value just uses return, not return nothing.

If I understand you correctly then this isn't a valid transformation because the value of the complete if block isn't nothing just because there are no return inside.

[jariji]

Are you asking about this function?

function foo1(cond)
    if cond
        1
    end
end

This code has a hidden nothing branch and I don't like that, so I would write it as

function foo1(cond)
    if cond
        1
    else
        nothing
    end
end

[fredrikekre]

Yes that's what I meant, and I agree that adding the else branch makes the nothing return value more obvious.

[jariji]

Just to show what I mean, here's some more code.

A function with only one (1) exit point doesn't need return.

function f(x)
  y = x + 1
  x + y
end

A function in which at least one (>=1) exit point returns a value, all exit points should return a value explicitly, including return nothing.

function f(xs)
  if isempty(xs)
    # Explicitly return `nothing` here.
    return nothing
  end
  println("hello") # side effect in branch
  return Some(first(xs))
end

A function in which no branch (0) returns a value just uses return, not return nothing.

function f(xs)
    while true
        if isempty(xs)
            return
        end
        pop!(xs)        
    end    
end

[KristofferC]

A function with only one (1) exit point doesn't need return.

I disagree with this. If for example the last statement is a function call it is unclear if it's value is indented to be returned or if it just happens to be the last statement.

[jariji]

A function's return value is an important part of its contract, so I don't think it should be left unclear. However, leaving the bare expression at the end doesn't prevent accidentally returning values, so adding return wouldn't help me - it's just adding verbosity.

When I don't want to return the last value, I will add nothing at the end to ensure no (possibly aliased) values are accidentally returned.

When I do want to return a value at the end, I just leave it. It's concise and effective.

[tecosaur]

Just to add another perspective into the mix, I'm rather attached to expression-oriented over statement-oriented programming.

In my codebases, in line with this, I only use return for early returns. In the rare case that I don't want to return anything (usually I like my functions to do work, and give the result), I explicitly leave a nothing at the end of the function expression.

Thoughts on the `if ... end` example

I much prefer having

function preferred(args...)
    # some work...
    if cond value end # I know people have thoughts on inline if statements too, but I like them when they're short
end

over

function disliked(args...)
    # some work...
    if cond
        return value
    else
        return nothing
    end
end

I find arguments[1] around accidental[2] return values somewhat convincing, but would find it a pity if this resulted in autoformatting[3] that structured Julia code more like a statement-oriented language despite Julia being largely expression-oriented by design.

[1]: Some earlier discussion on the topic, https://groups.google.com/g/julia-users/c/4RVR8qQDrUg
[2]: I've seen this described as an "implicit return", but in a (mostly) expression-based language this is the wrong mental model I'd argue.
[3]: Personally, I'd actually like it if there was a rule that removed redundant final returns.

[tecosaur]

Just FYI, for my own usage I'll be using a fork of Runic with this rule disabled.