Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Color incompletely specified type parameters in @code_warntype differently #41251

Open
Seelengrab opened this issue Jun 17, 2021 · 3 comments
Open

Comments

@Seelengrab
Copy link
Contributor

Seelengrab commented Jun 17, 2021

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
Copy link
Contributor Author

Seelengrab commented Jun 17, 2021

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

image

@simeonschaub
Copy link
Member

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
Copy link
Contributor Author

Seelengrab commented Jun 17, 2021

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants