Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inference: more thorough UnionAll renaming in apply_type_tfunc #48662

Merged
merged 1 commit into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1741,6 +1741,17 @@ const _tvarnames = Symbol[:_A, :_B, :_C, :_D, :_E, :_F, :_G, :_H, :_I, :_J, :_K,
canconst = true
tparams = Any[]
outervars = TypeVar[]

# first push the tailing vars from headtype into outervars
outer_start, ua = 0, headtype
while isa(ua, UnionAll)
if (outer_start += 1) > largs
push!(outervars, ua.var)
end
ua = ua.body
end
outer_start = outer_start - largs + 1

varnamectr = 1
ua = headtype
for i = 1:largs
Expand All @@ -1759,9 +1770,16 @@ const _tvarnames = Symbol[:_A, :_B, :_C, :_D, :_E, :_F, :_G, :_H, :_I, :_J, :_K,
uncertain = true
unw = unwrap_unionall(ai)
isT = isType(unw)
if isT && isa(ai,UnionAll) && contains_is(outervars, ai.var)
ai = rename_unionall(ai)
unw = unwrap_unionall(ai)
if isT
tai = ai
while isa(tai, UnionAll)
if contains_is(outervars, tai.var)
ai = rename_unionall(ai)
unw = unwrap_unionall(ai)
break
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
break

?

Copy link
Member Author

@N5N3 N5N3 Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, rename_unionall replaces all typevars in ai thus there's no need to re-rename?

Copy link
Sponsor Member

@vtjnash vtjnash Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay. That function is a bit awkward then, since it also exists in C without that behavior, and this is the only usage of the function anywhere (including not existing in any tests) from 93a725d

end
tai = tai.body
end
end
ai_w = widenconst(ai)
ub = ai_w isa Type && ai_w <: Type ? instanceof_tfunc(ai)[1] : Any
Expand Down Expand Up @@ -1822,7 +1840,7 @@ const _tvarnames = Symbol[:_A, :_B, :_C, :_D, :_E, :_F, :_G, :_H, :_I, :_J, :_K,
return Type{<:appl}
end
ans = Type{appl}
for i = length(outervars):-1:1
for i = length(outervars):-1:outer_start
ans = UnionAll(outervars[i], ans)
end
return ans
Expand Down
1 change: 1 addition & 0 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2743,6 +2743,7 @@ let 𝕃 = Core.Compiler.fallback_lattice
@test apply_type_tfunc(𝕃, Const(Issue47089), A, A) <: (Type{Issue47089{A,B}} where {A<:Integer, B<:Integer})
end
@test only(Base.return_types(keys, (Dict{String},))) == Base.KeySet{String, T} where T<:(Dict{String})
@test only(Base.return_types((r)->similar(Array{typeof(r[])}, 1), (Base.RefValue{Array{Int}},))) == Vector{T} where T<:(Array{Int})

# PR 27351, make sure optimized type intersection for method invalidation handles typevars

Expand Down