Limit the unioncomplexity instead of the unionlen in tmerge (#2…

…7417) The `unioncomplexity` is used as an estimate for the number of times `tmerge` had to be called to form the type.
JuliaLang · Jun 26, 2018 · bd13437 · bd13437
1 parent d494257
commit bd13437
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 9 deletions.
diff --git a/base/compiler/typelimits.jl b/base/compiler/typelimits.jl
@@ -4,7 +4,7 @@
 # limitation parameters #
 #########################
 
-const MAX_TYPEUNION_LEN = 4
+const MAX_TYPEUNION_COMPLEXITY = 3
 const MAX_INLINE_CONST_SIZE = 256
 
 #########################
@@ -346,12 +346,9 @@ function tmerge(@nospecialize(typea), @nospecialize(typeb))
         # XXX: this should never happen
         return Any
     end
-    # if we didn't start with any unions, then always OK to form one now
-    if !(typea isa Union || typeb isa Union)
-        # except if we might have switched Union and Tuple below, or would do so
-        if (isconcretetype(typea) && isconcretetype(typeb)) || !(typea <: Tuple && typeb <: Tuple)
-            return Union{typea, typeb}
-        end
+    # it's always ok to form a Union of two concrete types
+    if (isconcretetype(typea) || isType(typea)) && (isconcretetype(typeb) || isType(typeb))
+        return Union{typea, typeb}
     end
     # collect the list of types from past tmerge calls returning Union
     # and then reduce over that list
@@ -399,7 +396,7 @@ function tmerge(@nospecialize(typea), @nospecialize(typeb))
         end
     end
     u = Union{types...}
-    if unionlen(u) <= MAX_TYPEUNION_LEN
+    if unioncomplexity(u) <= MAX_TYPEUNION_COMPLEXITY
         # don't let type unions get too big, if the above didn't reduce it enough
         return u
     end
@@ -426,7 +423,7 @@ function tuplemerge(a::DataType, b::DataType)
     p = Vector{Any}(undef, lt + vt)
     for i = 1:lt
         ui = Union{ap[i], bp[i]}
-        if unionlen(ui) < MAX_TYPEUNION_LEN
+        if unioncomplexity(ui) < MAX_TYPEUNION_COMPLEXITY
             p[i] = ui
         else
             p[i] = Any

diff --git a/base/compiler/typeutils.jl b/base/compiler/typeutils.jl
@@ -124,3 +124,20 @@ function _switchtupleunion(t::Vector{Any}, i::Int, tunion::Vector{Any}, @nospeci
     end
     return tunion
 end
+
+# unioncomplexity estimates the number of calls to `tmerge` to obtain the given type by
+# counting the Union instances, taking also into account those hidden in a Tuple or UnionAll
+unioncomplexity(u::Union) = 1 + unioncomplexity(u.a) + unioncomplexity(u.b)
+function unioncomplexity(t::DataType)
+    t.name === Tuple.name || return 0
+    c = 0
+    for ti in t.parameters
+        ci = unioncomplexity(ti)
+        if ci > c
+            c = ci
+        end
+    end
+    return c
+end
+unioncomplexity(u::UnionAll) = max(unioncomplexity(u.body), unioncomplexity(u.var.ub))
+unioncomplexity(@nospecialize(x)) = 0
diff --git a/test/compiler/compiler.jl b/test/compiler/compiler.jl
@@ -1683,3 +1683,27 @@ struct Foo19668
     Foo19668(; kwargs...) = new()
 end
 @test Base.return_types(Foo19668, ()) == [Foo19668]
+
+# issue #27316 - inference shouldn't hang on these
+f27316(::Vector) = nothing
+f27316(::Any) = f27316(Any[][1]), f27316(Any[][1])
+@test Tuple{Nothing,Nothing} <: Base.return_types(f27316, Tuple{Int})[1] == Tuple{Union{Nothing, Tuple{Any,Any}},Union{Nothing, Tuple{Any,Any}}} # we may be able to improve this bound in the future
+function g27316()
+    x = nothing
+    while rand() < 0.5
+        x = (x,)
+    end
+    return x
+end
+@test Tuple{Tuple{Nothing}} <: Base.return_types(g27316, Tuple{})[1] == Any # we may be able to improve this bound in the future
+const R27316 = Tuple{Tuple{Vector{T}}} where T
+h27316_(x) = (x,)
+h27316_(x::Tuple{Vector}) = (Any[x][1],)::R27316 # a UnionAll of a Tuple, not vice versa!
+function h27316()
+    x = [1]
+    while rand() < 0.5
+        x = h27316_(x)
+    end
+    return x
+end
+@test Tuple{Tuple{Vector{Int}}} <: Base.return_types(h27316, Tuple{})[1] == Union{Vector{Int}, Tuple{Any}} # we may be able to improve this bound in the future