reducedim allocates array of incorrect eltype

[blegat]

To compute the eltype of the array storing the result, reducedim does

julia/base/reducedim.jl

Line 119 in bce5bbe

Tr = typeof(z) == typeof(x) && !isbits(T) ? T : typeof(z)

I don't understand the reasoning behind the condition typeof(z) == typeof(x) && !isbits(T). Why not just do Tr = typeof(z) ?

Here is a MWE:

struct Variable
    name::Symbol
end
struct AffExpr
    vars::Vector{Variable}
end
Base.zero(::Union{Variable, Type{Variable}, AffExpr}) = AffExpr(Variable[])
Base.:+(v::Variable, w::Variable) = AffExpr([v, w])
Base.:+(aff::AffExpr, v::Variable) = AffExpr([aff.vars; v])
Base.:+(aff1::AffExpr, aff2::AffExpr) = AffExpr([aff1.vars; aff2.vars])

The reduction fails because Tr is Variable instead of AffExpr. typeof(z) and typeof(x) are AffExpr and isbits(Variable) is false.

julia> sum([Variable(:x), Variable(:y)], 1)
ERROR: MethodError: Cannot `convert` an object of type AffExpr to an object of type Variable
This may have arisen from a call to the constructor Variable(...),
since type constructors fall back to convert methods.
Stacktrace:
 [1] fill!(::Array{Variable,1}, ::AffExpr) at ./multidimensional.jl:841
 [2] reducedim_initarray(::Array{Variable,1}, ::Int64, ::AffExpr, ::Type{Variable}) at ./reducedim.jl:73
 [3] _reducedim_init(::Base.#identity, ::Base.#+, ::Base.#zero, ::Base.#sum, ::Array{Variable,1}, ::Int64) at ./reducedim.jl:102
 [4] reducedim_init(::Function, ::Base.#+, ::Array{Variable,1}, ::Int64) at ./reducedim.jl:87
 [5] mapreducedim(::Function, ::Function, ::Array{Variable,1}, ::Int64) at ./reducedim.jl:242
 [6] sum(::Array{Variable,1}, ::Int64) at ./reducedim.jl:585
 [7] macro expansion at ./REPL.jl:97 [inlined]
 [8] (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:73

[JeffBezanson]

Agreed; I don't see why isbits would be relevant here.

[non-Jedi]

Spent a little time digging into this, though I think I'm in over my head at this point. The check for isbitstype is necessary for sum(Real[1 2 3; 3.5 4 5], 2) to work as expected. In such a case, Tr should be Real rather than Int64 like it ends up since zero(Real) == 0. I think what really needs to be checked there is whether T is an abstract or concrete type. Will PR something to that effect once I've given myself time to mentally untangle things a bit more.

As far as I can tell, the typeof(z) == typeof(x) check is useless. Why would you want Tr = false anyway?
For what it's worth, just substituting the below for that line passes all existing tests and makes the above MWE function correctly:

Tr = isconcretetype(T) ? typeof(z) : T

[nalimilan]

For the curious, this line comes from a generalization of a method which worked only for + (54e034a). That weird typeof(z) == typeof(x) check comes from that legacy, but it doesn't really make sense.

[non-Jedi]

Fixed by #28089

[blegat]

Indeed, thanks for the fix @nalimilan