fix part of #24166: bug in diagonal typevars on the right

also put a band-aid on reflexivity of diagonal vars
JuliaLang · Oct 17, 2017 · afffe8f · afffe8f
1 parent a54e60f
commit afffe8f
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 8 deletions.
diff --git a/src/subtype.c b/src/subtype.c
@@ -358,6 +358,10 @@ static jl_value_t *simple_join(jl_value_t *a, jl_value_t *b)
  return a;
  if (jl_is_kind(b) && jl_is_type_type(a) && jl_typeof(jl_tparam0(a)) == b)
  return b;
+ if (jl_is_typevar(a) && obviously_egal(b, ((jl_tvar_t*)a)->lb))
+ return a;
+ if (jl_is_typevar(b) && obviously_egal(a, ((jl_tvar_t*)b)->lb))
+ return b;
  if (!jl_has_free_typevars(a) && !jl_has_free_typevars(b)) {
  if (jl_subtype(a, b)) return b;
  if (jl_subtype(b, a)) return a;
@@ -633,18 +637,20 @@ static int subtype_unionall(jl_value_t *t, jl_unionall_t *u, jl_stenv_t *e, int8
  // ( Tuple{Int, Int} <: Tuple{T, T} where T) but
  // !( Tuple{Int, String} <: Tuple{T, T} where T)
  // Then check concreteness by checking that the lower bound is not an abstract type.
- if (ans && (vb.concrete || (!vb.occurs_inv && vb.occurs_cov > 1)) &&
- is_leaf_typevar((jl_value_t*)u->var)) {
- if (jl_is_typevar(vb.lb)) {
- // TODO test case that demonstrates the need for this?
- /*
+ int diagonal = !vb.occurs_inv && vb.occurs_cov > 1;
+ if (ans && (vb.concrete || (diagonal && is_leaf_typevar((jl_value_t*)u->var)))) {
+ if (vb.concrete && !diagonal && !is_leaf_bound(vb.ub)) {
+ // a non-diagonal var can only be a subtype of a diagonal var if its
+ // upper bound is concrete.
+ ans = 0;
+ }
+ else if (jl_is_typevar(vb.lb)) {
  jl_tvar_t *v = (jl_tvar_t*)vb.lb;
  jl_varbinding_t *vlb = lookup(e, v);
  if (vlb)
  vlb->concrete = 1;
- else // TODO handle multiple variables in vb.concretevar
- ans = (v == vb.concretevar);
- */
+ //else // TODO handle multiple variables in vb.concretevar
+ // ans = (v == vb.concretevar);
  }
  else if (!is_leaf_bound(vb.lb)) {
  ans = 0;

diff --git a/test/subtype.jl b/test/subtype.jl
@@ -116,6 +116,21 @@ function test_diagonal()
  # don't consider a diagonal variable concrete if it already has an abstract lower bound
  @test isequal_type(Tuple{Vararg{A}} where A>:Integer,
  Tuple{Vararg{A}} where A>:Integer)
+
+ # issue #24166
+ @test !issub(Tuple{T, T, Ref{T}} where T, Tuple{S, S, Ref{Q} where Q} where S)
+ @test !issub(Tuple{T, T, Ref{T}} where T, Tuple{S, S, Ref{Q} where Q} where S<:Integer)
+ @test !issub(Tuple{T, T, Ref{T}} where T, Tuple{S, S, Ref{Q} where Q} where S<:Int)
+ @test issub(Tuple{T, T, Ref{T}} where T<:Int, Tuple{S, S, Ref{Q} where Q} where S)
+ @test !issub(Tuple{T, T, Ref{T}} where T>:Int, Tuple{S, S, Ref{Q} where Q} where S)
+ @test !issub(Tuple{T, T, Ref{T}} where T>:Integer, Tuple{S, S, Ref{Q} where Q} where S)
+ @test !issub(Tuple{T, T, Ref{T}} where T>:Any, Tuple{S, S, Ref{Q} where Q} where S)
+
+ @test issub(Tuple{T, T} where Int<:T<:Int, Tuple{T, T} where Int<:T<:Int)
+ @test issub(Tuple{T, T} where T>:Int, Tuple{T, T} where T>:Int)
+ @test issub(Tuple{Tuple{T, T} where T>:Int}, Tuple{Tuple{T, T} where T>:Int})
+ @test issub(Tuple{Tuple{T, T} where T>:Int}, Tuple{Tuple{T, T}} where T>:Int)
+ @test issub(Tuple{Tuple{T, T}} where T>:Int, Tuple{Tuple{T, T} where T>:Int})
 end
 
 # level 3: UnionAll