fix #30114, specificity transitivity errors in convert methods (#30160)

JeffBezanson · web-flow · commit 6594acc254dd · 2018-12-04T21:29:32.000-05:00
diff --git a/NEWS.md b/NEWS.md
@@ -20,6 +20,10 @@ Language changes
     to the `Core` module ([#29968]).
   * Using the same name for both a local variable and a static parameter is now an error instead
     of a warning ([#29429]).
+  * Method signatures such as
+    `f(::Type{T}, ::T) where {T <: X}` and
+    `f(::Type{X}, ::Any)`
+    are now considered ambiguous. Previously a bug caused the first one to be considered more specific ([#30160]).
 
 Command-line option changes
 ---------------------------
diff --git a/base/bitarray.jl b/base/bitarray.jl
@@ -504,6 +504,7 @@ reinterpret(B::BitArray, dims::NTuple{N,Int}) where {N} = reshape(B, dims)
 
 if nameof(@__MODULE__) === :Base  # avoid method overwrite
 (::Type{T})(x::T) where {T<:BitArray} = copy(x)
+BitArray(x::BitArray) = copy(x)
 end
 
 """
diff --git a/src/subtype.c b/src/subtype.c
@@ -322,7 +322,7 @@ static int obviously_disjoint(jl_value_t *a, jl_value_t *b, int specificity)
         for(i=0; i < np; i++) {
             jl_value_t *ai = jl_tparam(ad,i);
             jl_value_t *bi = jl_tparam(bd,i);
-            if (!istuple && specificity) {
+            if (!istuple && specificity && jl_has_free_typevars(ai)) {
                 // X{<:SomeDataType} and X{Union{Y,Z,...}} need to be disjoint to
                 // avoid this transitivity problem:
                 // A = Tuple{Type{LinearIndices{N,R}}, LinearIndices{N}} where {N,R}
@@ -2585,9 +2585,8 @@ static int tuple_morespecific(jl_datatype_t *cdt, jl_datatype_t *pdt, int invari
             return 1;
 
         int cms = type_morespecific_(ce, pe, invariant, env);
-        int eqv = !cms && eq_msp(ce, pe, env);
 
-        if (!cms && !eqv && !sub_msp(ce, pe, env)) {
+        if (!cms && !sub_msp(ce, pe, env)) {
             /*
               A bound vararg tuple can be more specific despite disjoint elements in order to
               preserve transitivity. For example in
@@ -2600,7 +2599,8 @@ static int tuple_morespecific(jl_datatype_t *cdt, jl_datatype_t *pdt, int invari
         }
 
         // Tuple{..., T} not more specific than Tuple{..., Vararg{S}} if S is diagonal
-        if (eqv && i == clen-1 && clen == plen && !cva && pva && jl_is_typevar(ce) && jl_is_typevar(pe) && !cdiag && pdiag)
+        if (!cms && i == clen-1 && clen == plen && !cva && pva && eq_msp(ce, pe, env) &&
+            jl_is_typevar(ce) && jl_is_typevar(pe) && !cdiag && pdiag)
             return 0;
 
         if (cms) some_morespecific = 1;
@@ -2692,24 +2692,23 @@ static int num_occurs(jl_tvar_t *v, jl_typeenv_t *env)
     return 0;
 }
 
+#define HANDLE_UNIONALL_A                                               \
+    jl_unionall_t *ua = (jl_unionall_t*)a;                              \
+    jl_typeenv_t newenv = { ua->var, 0x0, env };                        \
+    newenv.val = (jl_value_t*)(intptr_t)count_occurs(ua->body, ua->var); \
+    return type_morespecific_(ua->body, b, invariant, &newenv)
+
+#define HANDLE_UNIONALL_B                                               \
+    jl_unionall_t *ub = (jl_unionall_t*)b;                              \
+    jl_typeenv_t newenv = { ub->var, 0x0, env };                        \
+    newenv.val = (jl_value_t*)(intptr_t)count_occurs(ub->body, ub->var); \
+    return type_morespecific_(a, ub->body, invariant, &newenv)
+
 static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_typeenv_t *env)
 {
     if (a == b)
         return 0;
 
-    if (jl_is_unionall(a)) {
-        jl_unionall_t *ua = (jl_unionall_t*)a;
-        jl_typeenv_t newenv = { ua->var, 0x0, env };
-        newenv.val = (jl_value_t*)(intptr_t)count_occurs(ua->body, ua->var);
-        return type_morespecific_(ua->body, b, invariant, &newenv);
-    }
-    if (jl_is_unionall(b)) {
-        jl_unionall_t *ub = (jl_unionall_t*)b;
-        jl_typeenv_t newenv = { ub->var, 0x0, env };
-        newenv.val = (jl_value_t*)(intptr_t)count_occurs(ub->body, ub->var);
-        return type_morespecific_(a, ub->body, invariant, &newenv);
-    }
-
     if (jl_is_tuple_type(a) && jl_is_tuple_type(b)) {
         // When one is JL_VARARG_BOUND and the other has fixed length,
         // allow the argument length to fix the tvar
@@ -2727,7 +2726,15 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
         return tuple_morespecific((jl_datatype_t*)a, (jl_datatype_t*)b, invariant, env);
     }
 
+    if (!invariant) {
+        if ((jl_datatype_t*)a == jl_any_type) return 0;
+        if ((jl_datatype_t*)b == jl_any_type && !jl_is_typevar(a)) return 1;
+    }
+
     if (jl_is_uniontype(a)) {
+        if (jl_is_unionall(b)) {
+            HANDLE_UNIONALL_B;
+        }
         // Union a is more specific than b if some element of a is more specific than b, but
         // not vice-versa.
         if (sub_msp(b, a, env))
@@ -2764,17 +2771,15 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
     }
 
     if (jl_is_uniontype(b)) {
+        if (jl_is_unionall(a)) {
+            HANDLE_UNIONALL_A;
+        }
         jl_uniontype_t *u = (jl_uniontype_t*)b;
         if (type_morespecific_(a, u->a, invariant, env) || type_morespecific_(a, u->b, invariant, env))
             return !type_morespecific_(b, a, invariant, env);
         return 0;
     }
 
-    if (!invariant) {
-        if ((jl_datatype_t*)a == jl_any_type) return 0;
-        if ((jl_datatype_t*)b == jl_any_type) return 1;
-    }
-
     if (jl_is_datatype(a) && jl_is_datatype(b)) {
         jl_datatype_t *tta = (jl_datatype_t*)a, *ttb = (jl_datatype_t*)b;
         // Type{Union{}} is more specific than other types, so TypeofBottom must be too
@@ -2796,13 +2801,20 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
                 for(size_t i=0; i < jl_nparams(tta); i++) {
                     jl_value_t *apara = jl_tparam(tta,i);
                     jl_value_t *bpara = jl_tparam(ttb,i);
-                    if (!jl_has_free_typevars(apara) && !jl_has_free_typevars(bpara) &&
-                        !jl_types_equal(apara, bpara))
+                    int afree = jl_has_free_typevars(apara);
+                    int bfree = jl_has_free_typevars(bpara);
+                    if (!afree && !bfree && !jl_types_equal(apara, bpara))
                         return 0;
-                    if (type_morespecific_(apara, bpara, 1, env))
+                    if (type_morespecific_(apara, bpara, 1, env) && (jl_is_typevar(apara) || !afree || bfree))
                         ascore += 1;
-                    else if (type_morespecific_(bpara, apara, 1, env))
+                    else if (type_morespecific_(bpara, apara, 1, env) && (jl_is_typevar(bpara) || !bfree || afree))
                         bscore += 1;
+                    else if (eq_msp(apara, bpara, env)) {
+                        if (!afree && bfree)
+                            ascore += 1;
+                        else if (afree && !bfree)
+                            bscore += 1;
+                    }
                     if (jl_is_typevar(bpara) && !jl_is_typevar(apara) && !jl_is_type(apara))
                         ascore1 = 1;
                     else if (jl_is_typevar(apara) && !jl_is_typevar(bpara) && !jl_is_type(bpara))
@@ -2828,9 +2840,6 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
                     return 0;
                 return ascore > bscore || adiag > bdiag;
             }
-            else if (invariant) {
-                return 0;
-            }
             tta = tta->super; super = 1;
         }
         return 0;
@@ -2852,16 +2861,18 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
         if (invariant) {
             if (((jl_tvar_t*)a)->ub == jl_bottom_type)
                 return 1;
-            if (jl_has_free_typevars(b)) {
-                if (type_morespecific_(((jl_tvar_t*)a)->ub, b, 0, env) ||
-                    eq_msp(((jl_tvar_t*)a)->ub, b, env))
-                    return num_occurs((jl_tvar_t*)a, env) >= 2;
-            }
-            else {
+            if (!jl_has_free_typevars(b))
                 return 0;
-            }
+            if (eq_msp(((jl_tvar_t*)a)->ub, b, env))
+                return num_occurs((jl_tvar_t*)a, env) >= 2;
+        }
+        else {
+            // need `{T,T} where T` more specific than `{Any, Any}`
+            if (b == (jl_value_t*)jl_any_type && ((jl_tvar_t*)a)->ub == (jl_value_t*)jl_any_type &&
+                num_occurs((jl_tvar_t*)a, env) >= 2)
+                return 1;
         }
-        return type_morespecific_((jl_value_t*)((jl_tvar_t*)a)->ub, b, 0, env);
+        return type_morespecific_(((jl_tvar_t*)a)->ub, b, 0, env);
     }
     if (jl_is_typevar(b)) {
         if (!jl_is_type(a))
@@ -2873,12 +2884,24 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
                 if (type_morespecific_(a, ((jl_tvar_t*)b)->ub, 0, env) ||
                     eq_msp(a, ((jl_tvar_t*)b)->ub, env))
                     return num_occurs((jl_tvar_t*)b, env) < 2;
+                return 0;
             }
             else {
+                if (obviously_disjoint(a, ((jl_tvar_t*)b)->ub, 1))
+                    return 0;
+                if (type_morespecific_(((jl_tvar_t*)b)->ub, a, 0, env))
+                    return 0;
                 return 1;
             }
         }
-        return type_morespecific_(a, (jl_value_t*)((jl_tvar_t*)b)->ub, 0, env);
+        return type_morespecific_(a, ((jl_tvar_t*)b)->ub, 0, env);
+    }
+
+    if (jl_is_unionall(a)) {
+        HANDLE_UNIONALL_A;
+    }
+    if (jl_is_unionall(b)) {
+        HANDLE_UNIONALL_B;
     }
 
     return 0;
diff --git a/test/specificity.jl b/test/specificity.jl
@@ -108,7 +108,7 @@ f17016(f, t::T_17016) = 0
 f17016(f, t1::Tuple) = 1
 @test f17016(0, (1,2,3)) == 0
 
-@test  args_morespecific(Tuple{Type{Any}, Any}, Tuple{Type{T}, Any} where T<:VecElement)
+@test !args_morespecific(Tuple{Type{Any}, Any}, Tuple{Type{T}, Any} where T<:VecElement)
 @test !args_morespecific((Tuple{Type{T}, Any} where T<:VecElement), Tuple{Type{Any}, Any})
 
 @test !args_morespecific(Tuple{Type{T}, Tuple{Any, Vararg{Any, N} where N}} where T<:Tuple{Any, Vararg{Any, N} where N},
@@ -240,3 +240,61 @@ end
 @test  args_morespecific(Tuple{Array,Int64}, Tuple{Array,Vararg{Int64,N}} where N)
 @test  args_morespecific(Tuple{Array,Int64}, Tuple{Array,Vararg{Int64,N} where N})
 @test !args_morespecific(Tuple{Array,Int64}, Tuple{AbstractArray, Array})
+
+# issue #30114
+let T1 = Tuple{Type{Tuple{Vararg{AbstractUnitRange{Int64},N} where N}},CartesianIndices{N,R} where R<:Tuple{Vararg{AbstractUnitRange{Int64},N}}} where N
+    T2 = Tuple{Type{T},T} where T<:AbstractArray
+    T3 = Tuple{Type{AbstractArray{T,N} where N},AbstractArray} where T
+    T4 = Tuple{Type{AbstractArray{T,N}},AbstractArray{s57,N} where s57} where N where T
+    @test !args_morespecific(T1, T2)
+    @test !args_morespecific(T1, T3)
+    @test !args_morespecific(T1, T4)
+    @test  args_morespecific(T2, T3)
+    @test  args_morespecific(T2, T4)
+end
+
+@test !args_morespecific(Tuple{Type{Tuple{Vararg{AbstractUnitRange{Int64},N}}},} where N,
+                         Tuple{Type{Tuple{Vararg{AbstractUnitRange,N} where N}},})
+
+@test  args_morespecific(Tuple{Type{SubArray{T,2,P} where T}, Array{T}} where T where P,
+                         Tuple{Type{AbstractArray{T,N} where N},AbstractArray} where T)
+
+# these are ambiguous
+@test !args_morespecific(Tuple{Type{T},T} where T<:BitArray,
+                         Tuple{Type{BitArray},Any})
+@test !args_morespecific(Tuple{Type{BitArray},Any},
+                         Tuple{Type{T},T} where T<:BitArray)
+
+abstract type Domain{T} end
+
+abstract type AbstractInterval{T} <: Domain{T} end
+
+struct Interval{L,R,T} <: AbstractInterval{T}
+end
+
+let A = Tuple{Type{Interval{:closed,:closed,T} where T}, Interval{:closed,:closed,T} where T},
+    B = Tuple{Type{II},                                  AbstractInterval} where II<:(Interval{:closed,:closed,T} where T),
+    C = Tuple{Type{AbstractInterval},                    AbstractInterval}
+    @test  args_morespecific(A, B)
+    @test !args_morespecific(B, C)
+    @test !args_morespecific(A, C)
+end
+
+let A = Tuple{Type{Domain},              Interval{L,R,T} where T} where R where L,
+    B = Tuple{Type{II},                  AbstractInterval} where II<:(Interval{:closed,:closed,T} where T),
+    C = Tuple{Type{AbstractInterval{T}}, AbstractInterval{T}} where T
+    @test !args_morespecific(A, B)
+    @test  args_morespecific(B, C)
+    @test !args_morespecific(A, C)
+end
+
+let A = Tuple{Type{AbstractInterval},    Interval{L,R,T} where T} where R where L,
+    B = Tuple{Type{II},                  AbstractInterval} where II<:(Interval{:closed,:closed,T} where T),
+    C = Tuple{Type{AbstractInterval{T}}, AbstractInterval{T}} where T
+    @test !args_morespecific(A, B)
+    @test  args_morespecific(B, C)
+    @test  args_morespecific(A, C)
+end
+
+@test args_morespecific(Tuple{Type{Missing},Any},
+                        Tuple{Type{Union{Nothing, T}},Any} where T)
