Base.Cartesian: add @ncallkw and allow else branch inside if

[matthias314]

This PR makes two changes to Base.Cartesian. Let me know if you prefer to have them separate.

Allow else branch inside if

Currently, the following gives an error:

julia> using Base.Cartesian
julia> @ntuple 3 i -> begin m = 0; if i == 1 ; m = 1 end ; m end   # expected: (1, 0, 0)
ERROR: LoadError: BoundsError: attempt to access 2-element Vector{Any} at index [3]
Stacktrace:
 [1] getindex
   @ Base ./essentials.jl:13 [inlined]
 [2] exprresolve(ex::Expr)
   @ Base.Cartesian ./cartesian.jl:408
 [3] exprresolve(ex::Expr)
   @ Base.Cartesian ./cartesian.jl:385
 [4] inlineanonymous(ex::Expr, val::Int64)
   @ Base.Cartesian ./cartesian.jl:246
 [5] var"@ntuple"(__source__::LineNumberNode, __module__::Module, N::Int64, ex::Any)
   @ Base.Cartesian ./cartesian.jl:200
in expression starting at REPL[2]:1

The reason is that the function exprresolve that tries to constant-evaluate expressions assumes that if statements always have an else branch. The PR fixes this.

(I actually think that many of the simplifications done by exprresolve would otherwise be done by the compiler, including the deletion of dead branches. The only exception seems to be the evaluation of constant array references. With tuples instead of arrays, this could also be handled by the compiler.)

New macro @ncallkw

This is like @ncall except for adding keyword arguments. This way you don't have to define a helper function to use keyword arguments. The keywords are given as the first macro argument as in

@ncallkw 3 f (a = 0, b = 1) x   # calls f(x_1, x_2, x_3; a = 0, b = 1)

Maybe you see a better approach. I've also tried to make @ncall call @ncallkw internally, but that caused problems during bootstrapping.

[matthias314]

Any thoughts about this? Whether it's a good idea to add another macro to Base.Cartesian is certainly debatable. On the other hand, fixing the error for if statements without else branch definitely seems worthwhile to me. Let me know if you want me to separate these two things.

[KristofferC]

Sorry for the late response here. I think this makes sense to me but to merge it I think it would be good to:

• Add an entry to it in the NEWS file.
• Fix the whitespace CI error.

[matthias314]

Thanks for the feedback. Added news entry and fixed whitespace issue.

Since submitting this PR, it has occurred to me that one could implement @ncallkw also by combining @ncall with Core.kwcall:

macro ncallkw(N::Int, f, kw, args...)
    esc(quote
        if isempty($kw)
            Base.Cartesian.@ncall($N, $f, $(args...))
        else
            Base.Cartesian.@ncall($N, Core.kwcall, $kw, $f, $(args...))
        end
    end)
end

If one doesn't mind using Core.kwcall, it's arguably simpler. Let me know if you prefer this version.