Use of variables defined inside if-else can poison inference

[thchr]

Consider the following function f:

function f()
    if rand(Bool)
        idxs = [[1,2],[1,2,3]]
    else
        idxs = [[1,2],[1,2,3]]
    end

    return idxs
end

As well as the functionally identical variation g:

function g()
    if rand(Bool)
        idxs = [[1,2],[1,2,3]]
    else
        idxs = [[1,2],[1,2,3]]
    end

    [idxs[j] for j in 1:length(idxs)]
    return idxs
end

Here, f will infer perfectly fine - but adding an iterator over idxs in g (which, notably, is not used for anything) will cause inference to give up, inferring Any as return value. Effectively, using the iterator poisons everything.

Defining and using variables this way is definitely an anti-pattern (and can be avoided in several ways) but sincef works just fine (and e.g., is not disallowed) it struck me as unintended that g does not.

[oscardssmith]

This is a bug. Can reproduce on master.

[simeonschaub]

This is just another duplicate of #15276.

[oscardssmith]

@simeonschaub how? This doesn't have a closure.

[simeonschaub]

Untyped array comprehensions are lowered using closures

[thchr]

Okay, reading parts of #15276, I guess I can see how this is related to capturing in closures if comprehensions are lowered via closures: thanks!

One thing I don't understand though is what the role of the if-else clause is here? E.g., the following variation infers fine (going by memory):

function h()
    idxs = Vector{Int}[]
    if rand(Bool)
        append!(idxs, [[1,2],[1,2,3]])
    else
        append!(idxs, [[1,2],[1,2,3]])
    end
    [idxs[j] for j in 1:length(idxs)]
    return idxs
end

[simeonschaub]

What matters is that there are multiple assignments to a variable that is captured by the closure. In your second example, idxs is only assigned once and then only ever gets mutated, so the type of idxs can never change.
In the original example, the assignments do happen to be inside mutually exclusive branches, so theoretically lowering could actually figure out that there's no need to box idxs in this particular case. I suspect this kind of analysis is probably sufficiently nontrivial to implement in our current logic though that we might as well just try to come up with a more general solution to #42996-like issues instead.