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

avoid invalidations when it doesn't resolve an MethodError #36733

Merged
merged 1 commit into from
Jul 30, 2020

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Jul 19, 2020

In these cases, the new method would not be called because the
call would still be an ambiguity error instead.

We also might observe that the method doesn't resolve an old MethodError
because it was previously covered by a different method.

(replaces #35877)

@timholy
Copy link
Sponsor Member

timholy commented Jul 20, 2020

Awesome! Thanks for tackling this.

I get a lot more invalidations with StaticArrays on this branch than with my tweaks to #35877 (I am not sure my tweaks are correct, so this is difficult to interpret). I tried deleting this method

convert(::Type{Union{}}, x) = throw(MethodError(convert, (Union{}, x)))

and that may have helped (I didn't really keep track), but I still get a lot of convert-related invalidations:

 inserting convert(::Type{SA}, x::Tuple) where SA<:StaticArray in StaticArrays at /home/tim/.julia/dev/StaticArrays/src/convert.jl:12 invalidated:
   backedges:  1: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{IOContext,Int64}, ::Tuple{IOContext,Int64}) (4 children)
               2: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Pkg.BinaryPlatforms.Platform,Bool}, ::Tuple{Pkg.BinaryPlatforms.Platform,Bool}) (4 children)
               3: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, String},Base.UUID}, ::Tuple{Union{Nothing, String},Base.UUID}) (4 children)
               4: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, String},Base.UUID,VersionNumber}, ::Tuple{Union{Nothing, String},Base.UUID,VersionNumber}) (4 children)
               5: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Any,Any,VersionNumber,Base.SHA1}, ::Tuple{Any,Any,VersionNumber,Base.SHA1}) (4 children)
               6: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, Base.UUID},Union{Nothing, String},Bool,Union{Nothing, VersionNumber},Union{Nothing, Base.SHA1}}, ::Tuple{Union{Nothing, Base.UUID},Union{Nothing, String},Bool,Union{Nothing, VersionNumber},Union{Nothing, Base.SHA1}}) (4 children)
               7: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{String,Any,Any}, ::Tuple{String,Any,Any}) (4 children)
               8: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{String,Union{typeof(Base.input_color), String},REPL.LineEditREPL,REPL.REPLCompletionProvider,REPL.var"#do_respond#58"{Bool,Bool,REPL.var"#66#76"{REPL.LineEditREPL},REPL.LineEditREPL,REPL.LineEdit.Prompt}}, ::Tuple{String,Union{typeof(Base.input_color), String},REPL.LineEditREPL,REPL.REPLCompletionProvider,REPL.var"#do_respond#58"{Bool,Bool,REPL.var"#66#76"{REPL.LineEditREPL},REPL.LineEditREPL,REPL.LineEdit.Prompt}}) (4 children)
               9: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{String,Union{typeof(Base.input_color), String},REPL.LineEditREPL,REPL.REPLCompletionProvider,typeof(REPL.return_callback)}, ::Tuple{String,Union{typeof(Base.input_color), String},REPL.LineEditREPL,REPL.REPLCompletionProvider,typeof(REPL.return_callback)}) (4 children)
              10: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Distributed.var"#208#211"{Distributed.WorkerPool},Any}, ::Tuple{Distributed.var"#208#211"{Distributed.WorkerPool},Any}) (4 children)
              11: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Pkg.Types.PreserveLevel,Pkg.BinaryPlatforms.Platform}, ::Tuple{Pkg.Types.PreserveLevel,Pkg.BinaryPlatforms.Platform}) (5 children)
              12: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Int64, Symbol}}, ::Tuple{Union{Int64, Symbol}}) (5 children)
              13: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Int64,Bool,Bool,Function}, ::Tuple{Int64,Bool,Bool,Function}) (5 children)
              14: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Int64, Symbol},Bool}, ::Tuple{Union{Int64, Symbol},Bool}) (5 children)
              15: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{IO,IO}, ::Tuple{IO,IO}) (5 children)
              16: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Pkg.BinaryPlatforms.Platform,Bool,Bool}, ::Tuple{Pkg.BinaryPlatforms.Platform,Bool,Bool}) (5 children)
              17: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, String},String,Dict{String,Base.UUID}}, ::Tuple{Union{Nothing, String},String,Dict{String,Base.UUID}}) (5 children)
              18: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{String, SubString},Dict{Symbol,Tuple{Ptr{Nothing},Any}}}, ::Tuple{Union{String, SubString},Dict{Symbol,Tuple{Ptr{Nothing},Any}}}) (5 children)
              19: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, String},Union{Nothing, Base.UUID},Any}, ::Tuple{Union{Nothing, String},Union{Nothing, Base.UUID},Any}) (5 children)
              20: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{String,Any}, ::Tuple{String,Any}) (5 children)
              21: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Any,Any,Any}, ::Tuple{Any,Any,Any}) (5 children)
              22: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, VersionNumber, Symbol},Union{Nothing, VersionNumber, Symbol},Pkg.BinaryPlatforms.CompilerABI}, ::Tuple{Union{Nothing, VersionNumber, Symbol},Union{Nothing, VersionNumber, Symbol},Pkg.BinaryPlatforms.CompilerABI}) (5 children)
              23: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, VersionNumber, Symbol},Union{Nothing, VersionNumber, Symbol},Union{Nothing, VersionNumber, Symbol}}, ::Tuple{Union{Nothing, VersionNumber, Symbol},Union{Nothing, VersionNumber, Symbol},Union{Nothing, VersionNumber, Symbol}}) (5 children)
              24: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, Base.UUID},Union{Nothing, String},Union{Nothing, VersionNumber},Union{Nothing, Base.SHA1}}, ::Tuple{Union{Nothing, Base.UUID},Union{Nothing, String},Union{Nothing, VersionNumber},Union{Nothing, Base.SHA1}}) (6 children)
              25: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, VersionNumber},Union{Nothing, VersionNumber},Union{Nothing, Symbol}}, ::Tuple{Union{Nothing, VersionNumber},Union{Nothing, VersionNumber},Union{Nothing, Symbol}}) (6 children)
              26: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Any,Int64}, ::Tuple{Any,Int64}) (7 children)
              27: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, LibGit2.GitAnnotated}}, ::Tuple{Union{Nothing, LibGit2.GitAnnotated}}) (8 children)
              28: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, String},Union{Pkg.Types.UpgradeLevel, VersionNumber, Pkg.Types.VersionSpec},Bool,Union{Nothing, Base.SHA1},Union{Nothing, String},Pkg.Types.GitRepo}, ::Tuple{Union{Nothing, String},Union{Pkg.Types.UpgradeLevel, VersionNumber, Pkg.Types.VersionSpec},Bool,Union{Nothing, Base.SHA1},Union{Nothing, String},Pkg.Types.GitRepo}) (9 children)
              29: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{String,Union{typeof(Base.input_color), String},REPL.LineEditREPL,REPL.ShellCompletionProvider,REPL.var"#do_respond#58"{Bool,Bool,REPL.var"#67#77"{REPL.LineEditREPL},REPL.LineEditREPL,REPL.LineEdit.Prompt}}, ::Tuple{String,Union{typeof(Base.input_color), String},REPL.LineEditREPL,REPL.ShellCompletionProvider,REPL.var"#do_respond#58"{Bool,Bool,REPL.var"#67#77"{REPL.LineEditREPL},REPL.LineEditREPL,REPL.LineEdit.Prompt}}) (9 children)
              30: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Bool,Union{Int64, Symbol}}, ::Tuple{Bool,Union{Int64, Symbol}}) (10 children)
              31: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Base.UUID,Union{Nothing, String},Union{Nothing, String},Bool,Pkg.Types.GitRepo,Union{Nothing, Base.SHA1},Union{Nothing, VersionNumber, Pkg.Types.VersionSpec}}, ::Tuple{Base.UUID,Union{Nothing, String},Union{Nothing, String},Bool,Pkg.Types.GitRepo,Union{Nothing, Base.SHA1},Union{Nothing, VersionNumber, Pkg.Types.VersionSpec}}) (10 children)
              32: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, Int64}}, ::Tuple{Union{Nothing, Int64}}) (10 children)
              33: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Any,Any}, ::Tuple{Any,Any}) (11 children)
              34: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Exception}, ::Tuple{Exception}) (11 children)
              35: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Expr, Module},Union{Expr, QuoteNode},Expr,Union{Expr, String},Union{Int64, Expr},Vector{Any}}, ::Tuple{Union{Expr, Module},Union{Expr, QuoteNode},Expr,Union{Expr, String},Union{Int64, Expr},Vector{Any}}) (11 children)
              36: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{SubString{String}, String},Dict{Symbol,Tuple{Ptr{Nothing},Any}}}, ::Tuple{Union{SubString{String}, String},Dict{Symbol,Tuple{Ptr{Nothing},Any}}}) (12 children)
              37: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Module,Union{Expr, QuoteNode},Expr,String,Int64,Vector{Any}}, ::Tuple{Module,Union{Expr, QuoteNode},Expr,String,Int64,Vector{Any}}) (13 children)
              38: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Type}, ::Tuple{Type}) (16 children)
              39: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Union{Nothing, Symbol},Pkg.BinaryPlatforms.CompilerABI}, ::Tuple{Union{Nothing, Symbol},Pkg.BinaryPlatforms.CompilerABI}) (16 children)
              40: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{IOContext}, ::Tuple{IOContext}) (18 children)
              41: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Function}, ::Tuple{Function}) (19 children)
              42: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{IO}, ::Tuple{IO}) (20 children)
              43: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Base.UUID,String,Union{Nothing, String},Pkg.Types.GitRepo,Bool,Union{Nothing, Base.SHA1},Union{Nothing, VersionNumber, Pkg.Types.VersionSpec}}, ::Tuple{Base.UUID,String,Union{Nothing, String},Pkg.Types.GitRepo,Bool,Union{Nothing, Base.SHA1},Union{Nothing, VersionNumber, Pkg.Types.VersionSpec}}) (21 children)
              44: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Pkg.BinaryPlatforms.Platform}, ::Tuple{Pkg.BinaryPlatforms.Platform}) (22 children)
              45: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{String,Base.UUID,Union{Nothing, String},String}, ::Tuple{String,Base.UUID,Union{Nothing, String},String}) (22 children)
              46: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{String,Union{Int64, Symbol}}, ::Tuple{String,Union{Int64, Symbol}}) (24 children)
              47: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Any}, ::Tuple{Any}) (38 children)
              48: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Integer}, ::Tuple{Integer}) (51 children)
              49: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Int64,Any}, ::Tuple{Int64,Any}) (80 children)
              50: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Int64,Bool,Function}, ::Tuple{Int64,Bool,Function}) (151 children)
              51: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type{_A} where _A<:Tuple{Any,Bool}, ::Tuple{Any,Bool}) (179 children)
              52: superseding convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311 with MethodInstance for convert(::Type, ::Tuple{Any,Vector{Union{Ptr{Nothing}, Base.InterpreterIP}}}) (305 children)

And that method is the one that would have been called,

julia> typeintersect(StaticArray, Base.AtLeast1)
Union{}

julia> convert(Union{}, (3,))
ERROR: MethodError: convert(::Type{Union{}}, ::Tuple{Int64}) is ambiguous. Candidates:
  convert(::Type{T}, arg) where T<:VecElement in Base at baseext.jl:8
  convert(::Type{T}, x::Tuple{Any,Vararg{Any,N} where N}) where T<:Tuple{Any,Vararg{Any,N} where N} in Base at essentials.jl:311
  convert(::Type{SA}, x::Tuple) where SA<:StaticArray in StaticArrays at /home/tim/.julia/dev/StaticArrays/src/convert.jl:12
Possible fix, define
  convert(::Type{Union{}}, ::Tuple{Any,Vararg{Any,N} where N})
Stacktrace:
 [1] top-level scope
   @ REPL[9]:1

To edit a specific method, type the corresponding number into the REPL and press Ctrl+Q

so it seems reasonable that this should have fixed it?

FYI altogether about 1200 MethodInstances are invalidated by loading StaticArrays.

@timholy
Copy link
Sponsor Member

timholy commented Jul 22, 2020

BTW, if you've not seen it, instructions on using SnoopCompile's analysis can be found here.

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Jul 23, 2020

I wonder if part of the problem will be that the type-system seems to be unsound with respect to evaluating them:

julia> typeintersect(Tuple{Int}, Tuple{Float64})
Union{}

julia> typeintersect(typeintersect(Tuple{Union{}}, Tuple{Int}), Tuple{Float64})
Tuple{Union{}}

But you're absolutely right that I was being too conservative, and can be possible at least some of those.

@timholy
Copy link
Sponsor Member

timholy commented Jul 23, 2020

Don't know if you've already checked this yourself, but with StaticArrays most of those convert invalidations have now become invalidations of

(::Type{T})(x::Tuple) where {T<:Tuple} = convert(T, x) # still use `convert` for tuples

due to https://github.com/JuliaArrays/StaticArrays.jl/blob/ad583c99768f3a381ba20b1a0782c9ca50890b50/src/FieldArray.jl#L113

The total number of unique invalidations is still 1200.

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Jul 24, 2020

Hadn't really looked. Though we're potentially running up against the wall where we're doing bad things to the actual latency by reducing the number here. But I'll take a look if I missed something else obvious.

@timholy
Copy link
Sponsor Member

timholy commented Jul 25, 2020

Since

julia> using StaticArrays

julia> typeintersect(FieldArray, Tuple)
Union{}

naively it seems more of the same?

@timholy
Copy link
Sponsor Member

timholy commented Jul 25, 2020

running up against the wall

To me it seems like a very squishy wall, with lots of room for uncertainty anywhere near it. If there is a mechanism by which I can systematically invalidate MethodInstance mi with code that loads in less time than it takes to recompile mi, then I can design a benchmark in which my code-loading invalidates it n times, recompiling in between each time, and thereby make the cost as bad as I want. (A real-world example is Base.require, where invalidating it forces recompilation before the next package gets loaded. It's not cheap to compile, so leaving open "holes" by which it can be repeatedly invalidated can be costly.) Conversely if loading my code takes longer than recompiling all the methodinstances it invalidates, then invalidation is not a big worry and indeed slowing the method-insertion process down might be counter-productive.

In other words, since so much depends on the specifics of what is being invalidated and its patterns of usage, I think you can probably design a benchmark to get any result you want.

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Jul 27, 2020

I think the ones triggered by that method addition are correct, given the shape of the ambiguity added. The shape of the effect of that new definition is:

julia> Base._methods_by_ftype(Tuple{Type{<:Tuple{IOContext}}, Tuple{IOContext}}, -1, Base.get_world_counter(), true, Ref(typemin(UInt64)), Ref(typemax(UInt64)), Ref(Int32(0)))
1-element Vector{Any}:
 Core.MethodMatch(Tuple{Type{var"#s1"} where var"#s1"<:Tuple{IOContext},Tuple{IOContext}}, svec(var"#s1"<:Tuple{IOContext}), (::Type{T})(x::Tuple) where T<:Tuple in Base at tuple.jl:249, true)

julia> (::Type{FA})(x::Tuple{Vararg{Any, N}}) where {N, FA <: AbstractString} = 1

julia> Base._methods_by_ftype(Tuple{Type{<:Tuple{IOContext}}, Tuple{IOContext}}, -1, Base.get_world_counter(), true, Ref(typemin(UInt64)), Ref(typemax(UInt64)), Ref(Int32(0)))
3-element Vector{Any}:
 Core.MethodMatch(Tuple{Type{Union{}},Tuple{IOContext}}, svec(1, Union{}), (::Type{FA})(x::Tuple{Vararg{Any,N}}) where {N, FA<:AbstractString} in Main at REPL[2]:1, false)
 Core.MethodMatch(Tuple{Type{var"#s1"} where var"#s1"<:Tuple{IOContext},Tuple{IOContext}}, svec(var"#s1"<:Tuple{IOContext}), (::Type{T})(x::Tuple) where T<:Tuple in Base at tuple.jl:249, true)
 Core.MethodMatch(Tuple{Type{var"#s1"} where var"#s1"<:Tuple{IOContext},Tuple{IOContext}}, svec(var"#s1"<:Tuple{IOContext}), (::Type{T})(itr) where T<:Tuple in Base at tuple.jl:254, true)

So there's a new method ambiguity that didn't exist before which potentially requires us to reconsider inlining decisions. (This was wrong in v1.4, prior to my unsorted-method-table work)

In these cases, the new method would not be called because the
call would still be an ambiguity error instead.

We also might observe that the method doesn't resolve an old MethodError
because it was previously covered by a different method.
@timholy
Copy link
Sponsor Member

timholy commented Jul 30, 2020

I'm still stuck on this:

julia> abstract type AT end

julia> (::Type{T})(x::Tuple) where {T<:AT} = nothing

julia> Union{}((1,2))
ERROR: MethodError: Union{}(::Tuple{Int64,Int64}) is ambiguous. Candidates:
  (::Type{T})(x::Tuple) where T<:AT in Main at REPL[2]:1
  (::Type{T})(x::Tuple) where T<:Tuple in Base at tuple.jl:249
  (::Type{T})(itr) where T<:Tuple in Base at tuple.jl:254
Possible fix, define
  Union{}(::Tuple)
Stacktrace:
 [1] top-level scope
   @ REPL[3]:1

Since that's a MethodError (cf title of this PR), it seems like a candidate for avoiding invalidation?

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Jul 30, 2020

It wasn't a MethodError before:

julia> @which Union{}((1,2))
(::Type{T})(x::Tuple) where T<:Tuple in Base at tuple.jl:249

So it's a new invalidation, not a resolution of one.

@vtjnash vtjnash merged commit f41115b into master Jul 30, 2020
@vtjnash vtjnash deleted the jn/ambiguousinvalidate branch July 30, 2020 20:27
simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 11, 2020
…#36733)

In these cases, the new method would not be called because the
call would still be an ambiguity error instead.

We also might observe that the method doesn't resolve an old MethodError
because it was previously covered by a different method.
vtjnash added a commit that referenced this pull request Jan 21, 2021
Error introduced by #36733

Fixes 38435
vtjnash added a commit that referenced this pull request Jan 25, 2021
KristofferC pushed a commit that referenced this pull request Jan 26, 2021
Error introduced by #36733

Fixes #38435

(cherry picked from commit cdaf740)
KristofferC pushed a commit that referenced this pull request Feb 1, 2021
Error introduced by #36733

Fixes #38435

(cherry picked from commit cdaf740)
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
Error introduced by #36733

Fixes #38435

(cherry picked from commit cdaf740)
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

Successfully merging this pull request may close these issues.

2 participants