Skip to content

Commit

Permalink
inference: fix widenconst call for ReturnNode of PartialStruct
Browse files Browse the repository at this point in the history
Previously, we might accidentally leave behind content in the fields
that should not be there. For example:

```
julia> code_typed(() -> (TypeVar(:x),), (), optimize=false)
1-element Vector{Any}:
 CodeInfo(
    @ REPL[1]:1 within `#3'
1 ─ %1 = Main.TypeVar(:x)::Core.Compiler.PartialTypeVar(x, true, true)
│   %2 = Core.tuple(%1)::Core.PartialStruct(Tuple{TypeVar}, Any[Core.Compiler.PartialTypeVar(x, true, true)])
└──      return %2
) => Tuple{TypeVar}

julia> ans[1][1].rettype
Core.PartialStruct(Tuple{TypeVar}, Any[Core.Compiler.PartialTypeVar(x, true, true)])
```
  • Loading branch information
vtjnash committed Feb 10, 2021
1 parent 2465db2 commit 4f5b22e
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 7 deletions.
29 changes: 23 additions & 6 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1325,6 +1325,28 @@ function abstract_eval_ssavalue(s::SSAValue, src::CodeInfo)
return typ
end

function widenreturn(@nospecialize rt)
# only propagate information we know we can store
# and is valid and good inter-procedurally
rt = widenconditional(rt)
isa(rt, Const) && return rt
isa(rt, Type) && return rt
if isa(rt, PartialStruct)
fields = copy(rt.fields)
haveconst = false
for i in 1:length(fields)
a = widenreturn(fields[i])
if !haveconst && has_const_info(a)
# TODO: consider adding && const_prop_profitable(a) here?
haveconst = true
end
fields[i] = a
end
haveconst && return PartialStruct(rt.typ, fields)
end
return widenconst(rt)
end

# make as much progress on `frame` as possible (without handling cycles)
function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
@assert !frame.inferred
Expand Down Expand Up @@ -1404,12 +1426,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
end
elseif isa(stmt, ReturnNode)
pc´ = n + 1
rt = widenconditional(abstract_eval_value(interp, stmt.val, s[pc], frame))
if !isa(rt, Const) && !isa(rt, Type) && !isa(rt, PartialStruct)
# only propagate information we know we can store
# and is valid inter-procedurally
rt = widenconst(rt)
end
rt = widenreturn(abstract_eval_value(interp, stmt.val, s[pc], frame))
# copy limitations to return value
if !isempty(frame.pclimitations)
union!(frame.limitations, frame.pclimitations)
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1318,7 +1318,7 @@ function tuple_tfunc(atypes::Vector{Any})
x = atypes[i]
# TODO ignore singleton Const (don't forget to update cache logic if you implement this)
if !anyinfo
anyinfo = (!isa(x, Type) && !isvarargtype(x)) || isType(x)
anyinfo = has_const_info(x)
end
if isa(x, Const)
params[i] = typeof(x.val)
Expand Down
2 changes: 2 additions & 0 deletions base/compiler/typeutils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ function has_nontrivial_const_info(@nospecialize t)
return !isdefined(typeof(val), :instance) && !(isa(val, Type) && hasuniquerep(val))
end

has_const_info(@nospecialize x) = (!isa(x, Type) && !isvarargtype(x)) || isType(x)

# Subtyping currently intentionally answers certain queries incorrectly for kind types. For
# some of these queries, this check can be used to somewhat protect against making incorrect
# decisions based on incorrect subtyping. Note that this check, itself, is broken for
Expand Down

0 comments on commit 4f5b22e

Please sign in to comment.