Julia: Segfault using StaticArrays and broadcasting

Created on 31 Aug 2018  ยท  13Comments  ยท  Source: JuliaLang/julia

From https://github.com/JuliaArrays/StaticArrays.jl/issues/482, running

# test.jl
using LinearAlgebra
using StaticArrays

perp(v) = SVector(v[2], -v[1])

struct A{N,T}
    v::SVector{N,SVector{2,T}}

    function A{N,T}(X) where {N,T}
        new(normalize.(perp.(X)))
    end
end
A(X) = A{length(X),eltype(eltype(X))}(X)

X = rand(SVector{3,SVector{2,Float64}})
A(X)

we get

  | | |_| | | | (_| |  |  Version 1.0.0 (2018-08-08)
 _/ |\__'_|_|_|\__'_|  |  
|__/                   |

julia> versioninfo()
Julia Version 1.0.0
Commit 5d4eaca0c9 (2018-08-08 20:58 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: AMD Ryzen 7 1800X Eight-Core Processor
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.0 (ORCJIT, znver1)

(v1.0) pkg> st
    Status `~/.julia/environments/v1.0/Project.toml`
  [90137ffa] StaticArrays v0.8.3

julia> include("test.jl")

signal (11): Segmentation fault
in expression starting at /tmp/test.jl:17
convert at /home/schmrlng/.julia/packages/StaticArrays/Ze5H3/src/convert.jl:8 [inlined]
Type at /tmp/test.jl:11
Type at /tmp/test.jl:14
unknown function (ip: 0x7f694004da96)
jl_fptr_trampoline at /home/schmrlng/code/oss/julia-1.0/src/gf.c:1829
jl_apply_generic at /home/schmrlng/code/oss/julia-1.0/src/gf.c:2182
do_call at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:324
eval_value at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:428
eval_stmt_value at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:363 [inlined]
eval_body at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:686
jl_interpret_toplevel_thunk_callback at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:799
unknown function (ip: 0xfffffffffffffffe)
unknown function (ip: 0x7f69043219ff)
unknown function (ip: 0xffffffffffffffff)
jl_interpret_toplevel_thunk at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:808
jl_toplevel_eval_flex at /home/schmrlng/code/oss/julia-1.0/src/toplevel.c:787
jl_parse_eval_all at /home/schmrlng/code/oss/julia-1.0/src/ast.c:838
jl_load at /home/schmrlng/code/oss/julia-1.0/src/toplevel.c:821
include at ./boot.jl:317 [inlined]
include_relative at ./loading.jl:1038
include at ./sysimg.jl:29
jl_apply_generic at /home/schmrlng/code/oss/julia-1.0/src/gf.c:2182
include at ./client.jl:388
jl_fptr_trampoline at /home/schmrlng/code/oss/julia-1.0/src/gf.c:1829
jl_apply_generic at /home/schmrlng/code/oss/julia-1.0/src/gf.c:2182
do_call at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:324
eval_value at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:428
eval_stmt_value at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:363 [inlined]
eval_body at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:686
jl_interpret_toplevel_thunk_callback at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:799
unknown function (ip: 0xfffffffffffffffe)
unknown function (ip: 0x7f69495b9c2f)
unknown function (ip: 0xffffffffffffffff)
jl_interpret_toplevel_thunk at /home/schmrlng/code/oss/julia-1.0/src/interpreter.c:808
jl_toplevel_eval_flex at /home/schmrlng/code/oss/julia-1.0/src/toplevel.c:787
jl_toplevel_eval_in at /home/schmrlng/code/oss/julia-1.0/src/builtins.c:622
eval at ./boot.jl:319
jl_apply_generic at /home/schmrlng/code/oss/julia-1.0/src/gf.c:2182
eval_user_input at /home/schmrlng/code/oss/julia-1.0/usr/share/julia/stdlib/v1.0/REPL/src/REPL.jl:85
macro expansion at /home/schmrlng/code/oss/julia-1.0/usr/share/julia/stdlib/v1.0/REPL/src/REPL.jl:117 [inlined]
#28 at ./task.jl:259
jl_apply_generic at /home/schmrlng/code/oss/julia-1.0/src/gf.c:2182
jl_apply at /home/schmrlng/code/oss/julia-1.0/src/julia.h:1536 [inlined]
start_task at /home/schmrlng/code/oss/julia-1.0/src/task.c:268
unknown function (ip: 0xffffffffffffffff)
Allocations: 3652857 (Pool: 3652443; Big: 414); GC: 7
[1]    7461 segmentation fault (core dumped)  julia
inference
Source
picture schmrlng

Most helpful comment

I've reduced this to an example which does not depend on StaticArrays:

struct SVec1{T} <: AbstractVector{T}
    data::T
end

Base.getindex(v::SVec1, i::Int) = v.data
Base.size(a::SVec1) where L = tuple(1)

using Base.Broadcast: BroadcastStyle, AbstractArrayStyle, Broadcasted
struct StaticVectorStyle <: AbstractArrayStyle{1} end
StaticVectorStyle(::Val{1}) = StaticVectorStyle()
BroadcastStyle(::Type{<:SVec1}) = StaticVectorStyle()

# copy overload
function Base.copy(B::Broadcasted{StaticVectorStyle})
    flat = Broadcast.flatten(B); as = flat.args; f = flat.f
    _broadcast(f, as...)
end

@generated function _broadcast(f, a1::Float64, a2::SVec1)
    return quote
        T = Core.Compiler.return_type(f, Tuple{typeof(a1), eltype(typeof(a2))})
        @inbounds return SVec1{T}(f(a1, a2[1]))
    end
end

X = SVec1{SVec1{Float64}}(SVec1{Float64}(0.0))
foo(X) = 1.0 .* X
@show foo(X)
@show typeof(foo(X))
@show Base.return_types(foo, Tuple{typeof(X)})
@show foo(X) isa Base.return_types(foo, Tuple{typeof(X)})[1]

Running this gives

foo(X) = SVec1{Float64}[[0.0]]
typeof(foo(X)) = SVec1{SVec1{Float64}}
Base.return_types(foo, Tuple{typeof(X)}) = Any[SVec1{Any}]
foo(X) isa (Base.return_types(foo, Tuple{typeof(X)}))[1] = false

If _broadcast is not (pointlessly) made a @generated function, the inferred return type is SVec1{SVec1}, which is still incorrect. (Should be SVec1{<:SVec1}.)

Let me see whether I can get the broadcast machinery out of the picture...

picture martinholters on 14 Sep 2018
❤2

All 13 comments

Curiously if you call A(X) without defining perp, then define perp and call A(X) again, it magically works.

using LinearAlgebra
using StaticArrays

struct A{N,T}
    v::SVector{N,SVector{2,T}}

    function A{N,T}(X) where {N,T}
        new(normalize.(perp.(X)))
    end
end
A(X) = A{length(X),eltype(eltype(X))}(X)

X = rand(SVector{3,SVector{2,Float64}})
A(X) # undefvar error
perp(v) = SVector(v[2], -v[1])
A(X) # now it works
haampie picture haampie on 31 Aug 2018

There is something going wrong with the inference of the broadcast:

julia> using LinearAlgebra

julia> using StaticArrays

julia> X = rand(SVector{3,SVector{2,Float64}})
3-element SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}:
 [0.44029, 0.858698]  
 [0.0988428, 0.976235]
 [0.117725, 0.184207] 

julia> foo(X) = normalize.(X)
foo (generic function with 1 method)

julia> @code_warntype foo(X)
Body::SArray{Tuple{3},Any,1,3}
1 1 โ”€      (Base.ifelse)(false, 0, 3)                                                                                 โ”‚โ•ปโ•ทโ•ทโ•ทโ•ทโ•ทโ•ทโ•ทโ•ท materialize
  โ”‚   %2 = invoke StaticArrays._broadcast(LinearAlgebra.normalize::typeof(normalize), $(QuoteNode(Size(3,)))::Size{(3,)}, (Size(3,),)::Tuple{Size{(3,)}}, _2::SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3})::SArray{Tuple{3},Any,1,3}
  โ””โ”€โ”€      return %2                                                                                                  โ”‚         

julia> foo(X)
3-element SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}:
 [0.456261, 0.889846]
 [0.100734, 0.994913]
 [0.538511, 0.842619]

julia> foo(X) isa SArray{Tuple{3},Any,1,3}
false

The crash then results from working with the wrongly inferred result. E.g.

bar(X) = foo(X)[1]
bar(X)
martinholters picture martinholters on 31 Aug 2018

Note that normalize invokes a broadcast itself (the element-wise multiplication with the inverse norm uses broadcast), so the recursion limiting heuristics kick in. Indeed, forcing inference of normalize(X[1]) (e.g. by calling it) before inferring/calling foo(X) makes the error go away. But the final return type is incorrectly widened to SArray{Tuple{3},Any,1,3} instead of SArray{Tuple{3},<:Any,1,3}. Maybe related to the data actually being stored in an NTuple{_,Float64}, which is widened to NTuple{_,Any}?

martinholters picture martinholters on 31 Aug 2018

Trying to break down and minimize this further, I've noticed that the return value often changes to be of the inferred type, i.e. (outer) eltype Any when making innocent changes like doing operations one at a time instead of in a function. Looking at the broadcast code in StaticArrays, I believe the element type is determined using return_type. So it might be that we end up inferring the element type as Any in one context but SArray{Tuple{2},Float64,1,2} in another, leading to the discrepancy. So maybe we're just seeing the consequences of what is warned about here:

https://github.com/JuliaLang/julia/blob/49e58ba035ea8a02c29c21cf1f2204f3b20f8dff/base/compiler/tfuncs.jl#L1167-L1170

martinholters picture martinholters on 3 Sep 2018

Oh, yeah, you should never use return_type in cases where you might leak that information to the caller. This (wrong answers, and perhaps a segfault if you're lucky) would be the expected result of using type-inference to compute the eltype.

vtjnash picture vtjnash on 4 Sep 2018

I have a hard time condensing this down to a self-contained example, and StaticArrays is probably not the best test vehicle here, but I don't see why the same issue shouldn't show up when broadcasting over an empty array where asking return_type to choose the element type is legitimate (or at least accepted as the least problematic solution). So maybe we should try to err on the side of correctness more often here, even if it means some performance implications?

Under the assumption that inference may produce a tighter result once some nested recursive calls have been inferred individually, but should not widen its result, I've tried this patch:

--- a/base/compiler/tfuncs.jl
+++ b/base/compiler/tfuncs.jl
@@ -1193,7 +1193,7 @@ function return_type_tfunc(argtypes::Vector{Any}, vtypes::VarTable, sv::Inferenc
                         return Const(typeof(rt.val))
                     elseif issingletontype(rt) || rt === Bottom
                         # output type was known for certain
-                        return Const(rt)
+                        return (rt === Bottom || isconcretetype(rt)) ? Const(rt) : Type{<:rt}
                     elseif (isa(tt, Const) || isconstType(tt)) &&
                            (isa(aft, Const) || isconstType(aft))
                         # input arguments were known for certain

It does fix this issue and does not seem to break anything critical. Maybe the condition could even be relaxed to rt !== Any. Not sure whether the other branches might need the same change. @vtjnash, do you think this is a path worth pursuing? Then I'll try to cook up a self-contained test case and open a PR.

martinholters picture martinholters on 6 Sep 2018

I'm quite out of my depth in this discussion, but in addition to type inference correctness (/not segfaulting) there's also potential performance hits if type inference terminates before nailing down concrete types for everything. Does the patch above (inferring eltype as <:Any) allocate on these test cases? If so, is there a way to increase the recursion limiting heuristic bound or would it be better to reconsider how StaticArrays does broadcasting?

I was hoping that this patch in StaticArrays.jl would fix the segfault (it does, and passes pkg> test StaticArrays) as well as ensure complete type inference (it doesn't, see below), but in any case relying on typeof($(first(exprs))) is probably too brittle of an assumption.

diff --git a/src/broadcast.jl b/src/broadcast.jl
index 39bf91f..824e06c 100644
--- a/src/broadcast.jl
+++ b/src/broadcast.jl
@@ -126,7 +126,14 @@ scalar_getindex(x::Tuple{<: Any}) = x[1]
     end

     eltype_exprs = [t <: Union{AbstractArray, Ref} ? :(eltype($t)) : :($t) for t โˆˆ a]
-    newtype_expr = :(return_type(f, Tuple{$(eltype_exprs...)}))
+    isconcretetype_exprs = [:(isconcretetype($t)) for t โˆˆ eltype_exprs]
+    newtype_expr = isempty(exprs) ? :(return_type(f, Tuple{$(eltype_exprs...)})) : quote
+        if all(($(isconcretetype_exprs...),))
+            typeof($(first(exprs)))
+        else
+            return_type(f, Tuple{$(eltype_exprs...)})
+        end
+    end

     return quote
         @_inline_meta

resulting in

julia> @code_warntype A{3,Float64}(X)
Body::A{3,Float64}
5 1 โ”€ %1  = (Core.tuple)(X)::Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}      โ”‚โ•ป             broadcasted
  โ”‚   %2  = %new(Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}}, perp, %1, nothing)::Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}}
  โ”‚   %3  = (Core.tuple)(%2)::Tuple{Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}}}
  โ”‚         (Base.ifelse)(false, 0, 3)                                                     โ”‚โ”‚โ•ปโ•ทโ•ทโ•ทโ•ทโ•ทโ•ทโ•ทโ•ทโ•ทโ•ทโ•ท  instantiate
  โ”‚   %5  = %new(Base.OneTo{Int64}, 3)::Base.OneTo{Int64}                                  โ”‚โ”‚โ”‚โ”ƒโ”‚โ”‚โ”‚โ”‚โ”‚โ”‚โ”‚โ”‚โ”‚    combine_axes
  โ”‚   %6  = (Core.tuple)(%5)::Tuple{Base.OneTo{Int64}}                                     โ”‚โ”‚โ”‚โ”‚โ”ƒโ”‚โ”‚โ”‚โ”‚โ”‚โ”‚       broadcast_axes
  โ”‚   %7  = %new(Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Tuple{Base.OneTo{Int64}},typeof(normalize),Tuple{Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}}}}, LinearAlgebra.normalize, %3, %6)::Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Tuple{Base.OneTo{Int64}},typeof(normalize),Tuple{Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}}}}
  โ”‚   %8  = %new(getfield(Base.Broadcast, Symbol("##7#8")){Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}},getfield(Base.Broadcast, Symbol("##9#10")){getfield(Base.Broadcast, Symbol("##11#12"))},getfield(Base.Broadcast, Symbol("##13#14")){getfield(Base.Broadcast, Symbol("##15#16"))},getfield(Base.Broadcast, Symbol("##5#6")){getfield(Base.Broadcast, Symbol("##3#4"))}}, %2, getfield(Base.Broadcast, Symbol("##9#10")){getfield(Base.Broadcast, Symbol("##11#12"))}(getfield(Base.Broadcast, Symbol("##11#12"))()), getfield(Base.Broadcast, Symbol("##13#14")){getfield(Base.Broadcast, Symbol("##15#16"))}(getfield(Base.Broadcast, Symbol("##15#16"))()), getfield(Base.Broadcast, Symbol("##5#6")){getfield(Base.Broadcast, Symbol("##3#4"))}(getfield(Base.Broadcast, Symbol("##3#4"))()))::getfield(Base.Broadcast, Symbol("##7#8")){Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}},getfield(Base.Broadcast, Symbol("##9#10")){getfield(Base.Broadcast, Symbol("##11#12"))},getfield(Base.Broadcast, Symbol("##13#14")){getfield(Base.Broadcast, Symbol("##15#16"))},getfield(Base.Broadcast, Symbol("##5#6")){getfield(Base.Broadcast, Symbol("##3#4"))}}
  โ”‚   %9  = %new(getfield(Base.Broadcast, Symbol("##1#2")){Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Tuple{Base.OneTo{Int64}},typeof(normalize),Tuple{Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}}}},getfield(Base.Broadcast, Symbol("##7#8")){Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}},getfield(Base.Broadcast, Symbol("##9#10")){getfield(Base.Broadcast, Symbol("##11#12"))},getfield(Base.Broadcast, Symbol("##13#14")){getfield(Base.Broadcast, Symbol("##15#16"))},getfield(Base.Broadcast, Symbol("##5#6")){getfield(Base.Broadcast, Symbol("##3#4"))}}}, %7, %8)::getfield(Base.Broadcast, Symbol("##1#2")){Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Tuple{Base.OneTo{Int64}},typeof(normalize),Tuple{Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}}}},getfield(Base.Broadcast, Symbol("##7#8")){Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}},getfield(Base.Broadcast, Symbol("##9#10")){getfield(Base.Broadcast, Symbol("##11#12"))},getfield(Base.Broadcast, Symbol("##13#14")){getfield(Base.Broadcast, Symbol("##15#16"))},getfield(Base.Broadcast, Symbol("##5#6")){getfield(Base.Broadcast, Symbol("##3#4"))}}}
  โ”‚   %10 = invoke StaticArrays._broadcast(%9::getfield(Base.Broadcast, Symbol("##1#2")){Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Tuple{Base.OneTo{Int64}},typeof(normalize),Tuple{Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}}}},getfield(Base.Broadcast, Symbol("##7#8")){Base.Broadcast.Broadcasted{StaticArrays.StaticArrayStyle{1},Nothing,typeof(perp),Tuple{SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}}},getfield(Base.Broadcast, Symbol("##9#10")){getfield(Base.Broadcast, Symbol("##11#12"))},getfield(Base.Broadcast, Symbol("##13#14")){getfield(Base.Broadcast, Symbol("##15#16"))},getfield(Base.Broadcast, Symbol("##5#6")){getfield(Base.Broadcast, Symbol("##3#4"))}}}, $(QuoteNode(Size(3,)))::Size{(3,)}, (Size(3,),)::Tuple{Size{(3,)}}, _2::SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3})::SArray{Tuple{3},_1,1,3} where _1
  โ”‚   %11 = (isa)(%10, SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3})::Bool            โ”‚             
  โ””โ”€โ”€       goto #3 if not %11                                                             โ”‚             
  2 โ”€ %13 = ฯ€ (%10, SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3})                     โ”‚             
  โ””โ”€โ”€       goto #4                                                                        โ”‚             
  3 โ”€ %15 = (Base.convert)(SArray{Tuple{3},SArray{Tuple{2},Float64,1,2},1,3}, %10)::SArray{Tuple{3},_1,1,3} where _1
  โ””โ”€โ”€       goto #4                                                                        โ”‚             
  4 โ”„ %17 = ฯ† (#2 => %13, #3 => %15)::SArray{Tuple{3},_1,1,3} where _1                     โ”‚             
  โ”‚   %18 = %new(A{3,Float64}, %17)::A{3,Float64}                                          โ”‚             
  โ””โ”€โ”€       return %18

with A, X defined as above (where _1 in lines %10, %15, %17 is problematic).

schmrlng picture schmrlng on 7 Sep 2018

Your patch also evaluates the first element twice. I've open a PR with an alternative at JuliaArrays/StaticArrays.jl#495. It leads to the same incomplete inference as yours, however, which presently seems unavoidable for nested broadcasts.

EDIT: Note that base Arrays are affected by the same problem:

julia> X = [rand(3) for i in 1:3]
3-element Array{Array{Float64,1},1}:
 [0.127849, 0.334689, 0.304509]
 [0.556885, 0.216156, 0.899888]
 [0.650899, 0.783833, 0.960853]

julia> bar(x) = 1.0 .* x
bar (generic function with 1 method)

julia> foo(X) = bar.(X)
foo (generic function with 1 method)

julia> @code_warntype foo(X)
Body::Any
1 1 โ”€ %1 = (Core.tuple)(X)::Tuple{Array{Array{Float64,1},1}}                                                    โ”‚โ•ป         broadcasted
  โ”‚   %2 = (Base.arraysize)(X, 1)::Int64                                                                        โ”‚โ”‚โ•ปโ•ทโ•ทโ•ทโ•ท     instantiate
  โ”‚   %3 = (Base.slt_int)(%2, 0)::Bool                                                                          โ”‚โ”‚โ”‚โ•ปโ•ทโ•ทโ•ท      combine_axes
  โ”‚   %4 = (Base.ifelse)(%3, 0, %2)::Int64                                                                      โ”‚โ”‚โ”‚โ”‚โ”ƒโ”‚โ”‚โ”‚โ”‚โ”‚    broadcast_axes
  โ”‚   %5 = %new(Base.OneTo{Int64}, %4)::Base.OneTo{Int64}                                                       โ”‚โ”‚โ”‚โ”‚โ”‚โ”ƒโ”‚โ”‚โ”‚      axes
  โ”‚   %6 = (Core.tuple)(%5)::Tuple{Base.OneTo{Int64}}                                                           โ”‚โ”‚โ”‚โ”‚โ”‚โ”‚โ”ƒ         map
  โ”‚   %7 = %new(Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1},Tuple{Base.OneTo{Int64}},typeof(bar),Tuple{Array{Array{Float64,1},1}}}, bar, %1, %6)::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1},Tuple{Base.OneTo{Int64}},typeof(bar),Tuple{Array{Array{Float64,1},1}}}
  โ”‚   %8 = invoke Base.Broadcast.copy(%7::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1},Tuple{Base.OneTo{Int64}},typeof(bar),Tuple{Array{Array{Float64,1},1}}})::Any
  โ””โ”€โ”€      return %8                                                                                            โ”‚         

julia> foo(X)
3-element Array{Array{Float64,1},1}:
 [0.127849, 0.334689, 0.304509]
 [0.556885, 0.216156, 0.899888]
 [0.650899, 0.783833, 0.960853]

(Here, inference is even worse, it's not even Vector.) The problem goes away if bar is called (on a Vector{Float64}) before foo is called (inferred).

martinholters picture martinholters on 14 Sep 2018

I've reduced this to an example which does not depend on StaticArrays:

struct SVec1{T} <: AbstractVector{T}
    data::T
end

Base.getindex(v::SVec1, i::Int) = v.data
Base.size(a::SVec1) where L = tuple(1)

using Base.Broadcast: BroadcastStyle, AbstractArrayStyle, Broadcasted
struct StaticVectorStyle <: AbstractArrayStyle{1} end
StaticVectorStyle(::Val{1}) = StaticVectorStyle()
BroadcastStyle(::Type{<:SVec1}) = StaticVectorStyle()

# copy overload
function Base.copy(B::Broadcasted{StaticVectorStyle})
    flat = Broadcast.flatten(B); as = flat.args; f = flat.f
    _broadcast(f, as...)
end

@generated function _broadcast(f, a1::Float64, a2::SVec1)
    return quote
        T = Core.Compiler.return_type(f, Tuple{typeof(a1), eltype(typeof(a2))})
        @inbounds return SVec1{T}(f(a1, a2[1]))
    end
end

X = SVec1{SVec1{Float64}}(SVec1{Float64}(0.0))
foo(X) = 1.0 .* X
@show foo(X)
@show typeof(foo(X))
@show Base.return_types(foo, Tuple{typeof(X)})
@show foo(X) isa Base.return_types(foo, Tuple{typeof(X)})[1]

Running this gives

foo(X) = SVec1{Float64}[[0.0]]
typeof(foo(X)) = SVec1{SVec1{Float64}}
Base.return_types(foo, Tuple{typeof(X)}) = Any[SVec1{Any}]
foo(X) isa (Base.return_types(foo, Tuple{typeof(X)}))[1] = false

If _broadcast is not (pointlessly) made a @generated function, the inferred return type is SVec1{SVec1}, which is still incorrect. (Should be SVec1{<:SVec1}.)

Let me see whether I can get the broadcast machinery out of the picture...

martinholters picture martinholters on 14 Sep 2018
❤2

Eventually we'll probably try to fix this and instead guarantee that this doesn't get inferred. Removing the label, since currently, calls to Core.Compiler.return_type are allowed to inject bugs into your code, so this isn't a bug with inference.

vtjnash picture vtjnash on 14 Sep 2018

Would a solution in the spirit of https://github.com/JuliaLang/julia/issues/28981#issuecomment-419029000 be a viable middle ground? I.e. only infer return_type if it yields a concrete type? I'd guess that's where it's most valuable to have it inferred and also least likely to go wrong?

Meanwhile, this would be a quite self-contained reproducer:

struct SVec1{T}
    data::T
end

doit(f, args) = _doit(f, args...)
@generated function _doit(f, a::SVec1{E}) where E
    return quote
        T = Core.Compiler.return_type(f, Tuple{E})
        return SVec1{T}(f(a.data))
    end
end

bar(X::SVec1) = doit(bar, (X,))
bar(X::Float64) = X

X = SVec1{SVec1{Float64}}(SVec1{Float64}(0.0))
@show bar(X)
@show typeof(bar(X))
@show Base.return_types(bar, Tuple{typeof(X)})
@show bar(X) isa Base.return_types(bar, Tuple{typeof(X)})[1]

(With the same effect of removing the @generated as above.)

martinholters picture martinholters on 14 Sep 2018
👍1

It's up to the user to not use Core.Compiler.return_type in a way that bugs will be too likely to be visible. For example, see how Base.broadcast and Base.map uses it.

vtjnash picture vtjnash on 17 Sep 2018

I believe this has been fixed for a while.

schmrlng picture schmrlng on 11 Jul 2019
Was this page helpful?
0 / 5 - 0 ratings

Related issues

dpsanders picture dpsanders  ยท  3Comments
wilburtownsend picture wilburtownsend  ยท  3Comments
ararslan picture ararslan  ยท  3Comments
yurivish picture yurivish  ยท  3Comments
tkoolen picture tkoolen  ยท  3Comments