Skip to content

Commit

Permalink
Follow-up JuliaLang#46693 - Also exclude `Type{Tuple{Union{...}, ...}…
Browse files Browse the repository at this point in the history
…}` (JuliaLang#46723)

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

The check in JuliaLang#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 <[email protected]>

Co-authored-by: Jameson Nash <[email protected]>
Keno and vtjnash authored Sep 13, 2022
1 parent 8a59be6 commit 969193b
Showing 3 changed files with 29 additions and 5 deletions.
10 changes: 5 additions & 5 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
@@ -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
23 changes: 23 additions & 0 deletions base/compiler/typeutils.jl
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
@@ -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})

0 comments on commit 969193b

Please sign in to comment.