inference: make getfield_tfunc more robust for abstract PartialStruct #34541
Conversation
base/compiler/tfuncs.jl
Outdated
| elseif isa(s, PartialStruct) | ||
| elseif isa(s00, PartialStruct) | ||
| s01 = widenconst(s00) | ||
| s = unwrap_unionall(s01)::DataType |
There was a problem hiding this comment.
| s = unwrap_unionall(s01)::DataType | |
| s = s01::DataType |
currently this simplification would hold, but do we want to assume that?
There was a problem hiding this comment.
Yeah, I think this simplification just makes sense to anyone who knows the fact that we don't form PartialStruct for UnionAll ?
|
It turns out we might make abstract PartialStruct for Tuple (refs #39402), so reopening to add vararg handling to this also |
|
So is it possible to construct a case that fails with the current code? |
We're currently careful never to make these. But good to be careful? refs #34513
| elseif isa(s, PartialStruct) | ||
| elseif isa(s00, PartialStruct) | ||
| s = widenconst(s00) | ||
| sty = unwrap_unionall(s)::DataType |
There was a problem hiding this comment.
Just to make sure: is there any possibility where s::DataType doesn't hold ?
This check is good to have though, we may want to form PartialStruct for abstract types other than tuples in the future.
There was a problem hiding this comment.
Currently, no. That is why this PR has sat for so long untouched. But turns out to be fairly straightforward here to add (though I think the necessary type lattice changes may currently be difficult).
DilumAluthge
removed
the
merge me
…JuliaLang#34541) We're currently careful never to make these. But good to be careful? refs JuliaLang#34513
…JuliaLang#34541) We're currently careful never to make these. But good to be careful? refs JuliaLang#34513
We're currently careful never to make these. But good to be careful?
refs #34513