Fix specificity with bound varargs

[timholy]

In #11242 (comment), the following example was shown to exhibit a method-sorting bug:

julia> f{T,N}(::Array{T,N}, ::Vararg{Int, N}) = 1
f (generic function with 1 method)

julia> f(::Array, ::Int) = 2
f (generic function with 2 methods)

In cases where one signature is a bound-length vararg and the other has specified length, this PR fixes the length of the vararg before deciding specificity, provided that doing so also narrows the type of an earlier argument.

Something along these lines appears necessary to ensure that the specificity computation operates over just the domain of intersection of the two methods. Whether this is the right implementation is a separate question.

[JeffBezanson]

Yes, +1 to moving this code.

[timholy]

I'm wondering whether we want this in light of #16276 (comment). Perhaps we should force the user to write out the f(::Vector, ::Int) case specifically. (It will be annoying, of course.)

[vtjnash]

bump. this just needs the typo fix I mentioned above to JL_GC_PUSH2 to merge

[timholy]

OK, fixed. I was hesitant because, thinking in terms of a future world where subtyping and method specificity is decided in a diagonally-aware manner, I am somewhat skeptical that our current specificity heuristics will be sustainable. But this stage of julia-0.5 is not the time to worry about such things.

[yuyichao]

This seems to have caused a segfault

yuyichao% git bisect bad 
9614c404b1ce58afede84174759b40264fb24524 is the first bad commit
commit 9614c404b1ce58afede84174759b40264fb24524
Author: Tim Holy <[email protected]>
Date:   Thu Jun 23 20:29:33 2016 -0500

    Fix specificity with bound varargs

:040000 040000 d3e0bebe34a82615d2a77812723cd4ab1d4f3a65 321a3d3dd0b9cab27304b2da9ec4d535082416a9 M      src
:040000 040000 6d54ab6e5960766e77656587bf022168dc152a2b efdcde6199221cec35edbce92dbcff97e6695dcf M      test
yuyichao% git bisect log 
git bisect start
# bad: [68e1dd434a891a392144073d7f99f8c740eeffd1] Merge pull request #17089 from pkofod/densecat
git bisect bad 68e1dd434a891a392144073d7f99f8c740eeffd1
# good: [c20199e11a190dc27a762f7610cd1fc0f783073f] Merge pull request #17035 from JuliaLang/ksh/doclq
git bisect good c20199e11a190dc27a762f7610cd1fc0f783073f
# good: [26fec3463a9082b8ed72ad43feed28e28524d7ad] Merge pull request #16665 from jgrg/master
git bisect good 26fec3463a9082b8ed72ad43feed28e28524d7ad
# bad: [1a1a9a63f1b49f8f9e0b0905b834b5fa8d272983] Merge pull request #17081 from JuliaLang/yyc/gc/finalize-ptr
git bisect bad 1a1a9a63f1b49f8f9e0b0905b834b5fa8d272983
# bad: [e10bd496dbe66d99341643c20b5298b246c17762] Merge pull request #17085 from JuliaLang/teh/pda_params
git bisect bad e10bd496dbe66d99341643c20b5298b246c17762
# bad: [beab3b6b7891b6f496bd71d669da78a33c9563ee] Merge pull request #16249 from JuliaLang/teh/morespecific_vararg
git bisect bad beab3b6b7891b6f496bd71d669da78a33c9563ee
# bad: [9614c404b1ce58afede84174759b40264fb24524] Fix specificity with bound varargs
git bisect bad 9614c404b1ce58afede84174759b40264fb24524
# first bad commit: [9614c404b1ce58afede84174759b40264fb24524] Fix specificity with bound varargs

Repro
f(::Tuple) = nothing; f{N}(::Vararg{Tuple,N}) = nothing

Direct repro
ccall(:jl_args_morespecific, Void, (Any, Any), Tuple{Vararg{Tuple,TypeVar(:N, true)}}, Tuple{Tuple})

BT

(rr) bt
#0  0x00007f4ce4ae56dc in inst_tuple_w_ (t=0x7f4add9401f0, env=0x7ffef0659480, n=2, stack=0x0, check=1) at /home/yuyichao/projects/julia/master/src/jltypes.c:2246
#1  0x00007f4ce4ae5e7b in inst_type_w_ (t=0x7f4add9401f0, env=0x7ffef0659480, n=2, stack=0x0, check=1) at /home/yuyichao/projects/julia/master/src/jltypes.c:2332
#2  0x00007f4ce4ae6038 in inst_type_w_ (t=0x7f4ae1e2bc70, env=0x7ffef0659480, n=2, stack=0x0, check=0) at /home/yuyichao/projects/julia/master/src/jltypes.c:2345
#3  0x00007f4ce4ae59ba in inst_tuple_w_ (t=0x7f4ae1e22540, env=0x7ffef0659480, n=2, stack=0x0, check=1) at /home/yuyichao/projects/julia/master/src/jltypes.c:2276
#4  0x00007f4ce4ae5e7b in inst_type_w_ (t=0x7f4ae1e22540, env=0x7ffef0659480, n=2, stack=0x0, check=1) at /home/yuyichao/projects/julia/master/src/jltypes.c:2332
#5  0x00007f4ce4ae62fe in jl_instantiate_type_with (t=0x7f4ae1e22540, env=0x7ffef0659480, n=2) at /home/yuyichao/projects/julia/master/src/jltypes.c:2377
#6  0x00007f4ce4ae74bd in jl_fix_vararg_bound (tt=0x7f4ae1e22540, nfix=1) at /home/yuyichao/projects/julia/master/src/jltypes.c:2645
#7  0x00007f4ce4ae837f in jl_args_morespecific_fix1 (a=0x7f4ae1e22540, b=0x7f4ae1e21dd0, swap=0) at /home/yuyichao/projects/julia/master/src/jltypes.c:2904
#8  0x00007f4ce4ae857a in jl_args_morespecific (a=0x7f4ae1e22540, b=0x7f4ae1e21dd0) at /home/yuyichao/projects/julia/master/src/jltypes.c:2940

The issue seems to be a OOB access in inst_tuple_w_ on env[2] (it is looping from 0 to 3) even though jl_fix_vararg_bound only adds two elements in there. There's a comment

// Note this does not instantiate Tuple{Vararg{Int,3}}; that's done in
// jl_apply_tuple_type_v_

in inst_tuple_w_, not sure if it is related.

@timholy

[yuyichao]

Ref https://groups.google.com/forum/?fromgroups=#!topic/julia-users/FhKYJ6ogo6s

[timholy]

Thanks for the detective work as usual, @yuyichao.