JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.55k stars 5.47k forks source link

Color incompletely specified type parameters in `@code_warntype` differently #41251

Open Seelengrab opened 3 years ago

Seelengrab commented 3 years ago

When using @code_warntype to check for type instabilities, sometimes some instability is induced by accessing incompletely specified type parameters/fields:

julia> using StaticArrays

julia> struct Container{T} x::T end                    

julia> fetch(c::Container{T}) where T = c.x
fetch (generic function with 1 method)     

julia> function good_gen()                                                                                                    
           list = [ Good([i]) for i in 1:1000 ]                                                                               
           x -> [ fetch(x)*y for y ∈ list ]                                                                                   
       end                                                                                                                    
good_gen (generic function with 1 method)                                                                                    

julia> good_prod = good_gen()                                                                                                 
#66 (generic function with 1 method)                                                                                          

julia> m_bad = Container{Bad}(Bad([1]))                                                                                       
Container{SMatrix{1, 1, Int64}}([1])                                                                                          

julia> @code_warntype good_prod(m_bad)                                                                                        
MethodInstance for (::var"#66#69"{Vector{SMatrix{1, 1, Int64, 1}}})(::Container{SMatrix{1, 1, Int64}})                        
  from (::var"#66#69")(x) in Main at REPL[73]:3                                                                               
Arguments                                                                                                                     
  #self#::var"#66#69"{Vector{SMatrix{1, 1, Int64, 1}}}                                                                        
  x::Container{SMatrix{1, 1, Int64}}                                                                                          
Locals                                                                                                                        
  #67::var"#67#70"{Container{SMatrix{1, 1, Int64}}}                                                                           
Body::Vector                                                                                                                  
1 ─ %1 = Main.:(var"#67#70")::Core.Const(var"#67#70")                                                                         
│   %2 = Core.typeof(x)::Core.Const(Container{SMatrix{1, 1, Int64}})                                                          
│   %3 = Core.apply_type(%1, %2)::Core.Const(var"#67#70"{Container{SMatrix{1, 1, Int64}}})                                    
│        (#67 = %new(%3, x))                                                                                                  
│   %5 = #67::var"#67#70"{Container{SMatrix{1, 1, Int64}}}                                                                    
│   %6 = Core.getfield(#self#, :list)::Vector{SMatrix{1, 1, Int64, 1}}                                                        
│   %7 = Base.Generator(%5, %6)::Base.Generator{Vector{SMatrix{1, 1, Int64, 1}}, var"#67#70"{Container{SMatrix{1, 1, Int64}}}}
│   %8 = Base.collect(%7)::Vector                                                                                             
└──      return %8                                                                                                            

In the above @code_warntype, all instances where Container{SMatrix{1, 1, Int64}} is printed are blue, indicating type stability. Using that type and accessing its field however induces type instability, since its field is neither abstractly nor concretely typed. I think coloring that incompletely specified type parameter differently (yellow? red?) would be a good idea.

Seelengrab commented 3 years ago

Here's an example what the code above looks like right now:

image

simeonschaub commented 3 years ago

Containers with non-concretely typed fields do not introduce type instabilities per se. It is only if you access the field that a type instability is introduced and in this case code_warntype will warn you appropriately.

Seelengrab commented 3 years ago

Right, I'm aware of that. Type parameters are usually used to specify field types though, so having those incompletely specified will in most cases lead to type instabilities down the road, when accessing those fields (if the field is never accessed, why have it in the first place?). The problem is that this isn't apparent from @code_warntype using one of those fields (e.g. in this example, fetch doesn't even show up, masking the access of the field) - the functions and methods themselves are fine in any case and the problem lies with Container. Communicating that possibility in @code_warntype seems like a good idea.

Moreover, if you don't know that SMatrix has 4 type parameters, simply looking at @code_warntype leads to the confusing conclusion that the use is fine (as came up in this discourse post, which inspired this issue).