From 969193b9b315483240c1689beada894563a1d75c Mon Sep 17 00:00:00 2001
From: Keno Fischer <keno@juliacomputing.com>
Date: Mon, 12 Sep 2022 21:03:52 -0400
Subject: [PATCH] Follow-up #46693 - Also exclude `Type{Tuple{Union{...},
 ...}}` (#46723)

* Follow-up #46693 - Also exclude `Type{Tuple{Union{...}, ...}}`

The check in #46693 was attempting to guarantee that the runtime
value was a `DataType`, but was missing at least the case where
a tuple of unions could have been re-distributed. Fix that up
and refactor while we're at it.

* Apply suggestions from code review

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
---
 base/compiler/tfuncs.jl    | 10 +++++-----
 base/compiler/typeutils.jl | 23 +++++++++++++++++++++++
 test/compiler/inline.jl    |  1 +
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/base/compiler/tfuncs.jl b/base/compiler/tfuncs.jl
index a5b55c6fc3a8e..d93958ac16186 100644
--- a/base/compiler/tfuncs.jl
+++ b/base/compiler/tfuncs.jl
@@ -824,9 +824,8 @@ function getfield_nothrow(@nospecialize(s00), @nospecialize(name), boundscheck::
     if isa(s, Union)
         return getfield_nothrow(rewrap_unionall(s.a, s00), name, boundscheck) &&
                getfield_nothrow(rewrap_unionall(s.b, s00), name, boundscheck)
-    elseif isType(s)
-        sv = s.parameters[1]
-        s = s0 = typeof(sv)
+    elseif isType(s) && isTypeDataType(s.parameters[1])
+        s = s0 = DataType
     end
     if isa(s, DataType)
         # Can't say anything about abstract types
@@ -941,10 +940,11 @@ function _getfield_tfunc(@nospecialize(s00), @nospecialize(name), setfield::Bool
             s = typeof(sv)
         else
             sv = s.parameters[1]
-            if isa(sv, DataType) && isa(name, Const) && (!isType(sv) && sv !== Core.TypeofBottom)
+            if isTypeDataType(sv) && isa(name, Const)
                 nv = _getfield_fieldindex(DataType, name)
                 if nv == DATATYPE_NAME_FIELDINDEX
-                    # N.B. This doesn't work in general, because
+                    # N.B. This only works for fields that do not depend on type
+                    # parameters (which we do not know here).
                     return Const(sv.name)
                 end
                 s = DataType
diff --git a/base/compiler/typeutils.jl b/base/compiler/typeutils.jl
index 303cc6bd711a5..d2992fc6113ba 100644
--- a/base/compiler/typeutils.jl
+++ b/base/compiler/typeutils.jl
@@ -23,6 +23,29 @@ function hasuniquerep(@nospecialize t)
     return false
 end
 
+"""
+    isTypeDataType(@nospecialize t)
+
+For a type `t` test whether ∀S s.t. `isa(S, rewrap_unionall(Type{t}, ...))`,
+we have `isa(S, DataType)`. In particular, if a statement is typed as `Type{t}`
+(potentially wrapped in some UnionAll), then we are guaranteed that this statement
+will be a DataType at runtime (and not e.g. a Union or UnionAll typeequal to it).
+"""
+function isTypeDataType(@nospecialize t)
+    isa(t, DataType) || return false
+    isType(t) && return false
+    # Could be Union{} at runtime
+    t === Core.TypeofBottom && return false
+    if t.name === Tuple.name
+        # If we have a Union parameter, could have been redistributed at runtime,
+        # e.g. `Tuple{Union{Int, Float64}, Int}` is a DataType, but
+        # `Union{Tuple{Int, Int}, Tuple{Float64, Int}}` is typeequal to it and
+        # is not.
+        return _all(isTypeDataType, t.parameters)
+    end
+    return true
+end
+
 function has_nontrivial_const_info(lattice::PartialsLattice, @nospecialize t)
     isa(t, PartialStruct) && return true
     isa(t, PartialOpaque) && return true
diff --git a/test/compiler/inline.jl b/test/compiler/inline.jl
index 069aa420d3267..22796d76b9173 100644
--- a/test/compiler/inline.jl
+++ b/test/compiler/inline.jl
@@ -1511,4 +1511,5 @@ end
 
 # Test getfield modeling of Type{Ref{_A}} where _A
 @test Core.Compiler.getfield_tfunc(Type, Core.Compiler.Const(:parameters)) !== Union{}
+@test !isa(Core.Compiler.getfield_tfunc(Type{Tuple{Union{Int, Float64}, Int}}, Core.Compiler.Const(:name)), Core.Compiler.Const)
 @test fully_eliminated(Base.ismutable, Tuple{Base.RefValue})