inference: allow PartialStruct to have Union{} field to indicate strict undef
#57541
Conversation
aviatesk
force-pushed
the
avi/strict-undef-fields
branch
3 times, most recently
from
245049e to
c8dcd2c
Compare
|
The initial choice to make |
| fldcnt = fieldcount_noerror(objt)::Int | ||
| fields = Any[fieldtype(objt0, i) for i = 1:fldcnt] | ||
| if fields[fldidx] === Union{} | ||
| return nothing # refine this field |
There was a problem hiding this comment.
I'm not sure I understand the comment here, doesn't refining mean returning a PartialStruct? (though it makes sense to return nothing as a Union{} field type already implies undef[fldidx] === true)
There was a problem hiding this comment.
This PR wasn't finished yesterday, so the logic might have been messed up. Undef the refined definition of PartialStruct, fields[fldidx] === Union{} is a special case, meaning that field is guaranteed not to be initialized, so we need to bail out here.
aviatesk
force-pushed
the
avi/strict-undef-fields
branch
from
c8dcd2c to
1397a8b
Compare
aviatesk
changed the title
PartialStruct to have Union{} field to indicate strict undefPartialStruct to have Union{} field to indicate strict undef
| undef::BitVector # represents whether a given field may be undefined | ||
| undefs::Vector{Union{Nothing,Bool}} # represents whether a given field may be undefined |
There was a problem hiding this comment.
Renamed undef to undefs to avoid naming collision with the undef initializer.
Compiler/src/typelimits.jl
Outdated
| function refines_definedness_information(pstruct::PartialStruct) | ||
| nflds = length(pstruct.undef) | ||
| something(findfirst(pstruct.undef), nflds + 1) - 1 > datatype_min_ninitialized(pstruct.typ) | ||
| end | ||
|
|
||
| function define_field(pstruct::PartialStruct, fi::Int) | ||
| if !is_field_maybe_undef(pstruct, fi) | ||
| # no new information to be gained | ||
| return nothing | ||
| end | ||
|
|
||
| new = expand_partialstruct(pstruct, fi) | ||
| if new === nothing | ||
| new = PartialStruct(fallback_lattice, pstruct.typ, copy(pstruct.undef), copy(pstruct.fields)) | ||
| end | ||
| new.undef[fi] = false | ||
| return new | ||
| end | ||
|
|
||
| function expand_partialstruct(pstruct::PartialStruct, until::Int) | ||
| n = length(pstruct.undef) | ||
| until ≤ n && return nothing | ||
|
|
||
| undef = partialstruct_init_undef(pstruct.typ, until; all_defined = false) | ||
| for i in 1:n | ||
| undef[i] &= pstruct.undef[i] | ||
| end | ||
| nf = length(pstruct.fields) | ||
| typ = pstruct.typ | ||
| fields = Any[i ≤ nf ? pstruct.fields[i] : fieldtype(typ, i) for i in 1:until] | ||
| return PartialStruct(fallback_lattice, typ, undef, fields) | ||
| end |
There was a problem hiding this comment.
I've inlined some logic implemented here because it is now only used once in other files.
aviatesk
force-pushed
the
avi/strict-undef-fields
branch
from
1397a8b to
b00838b
Compare
|
@nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
|
|
||
| function Base.getproperty(pstruct::Core.PartialStruct, name::Symbol) | ||
| name === :undef && return getfield(pstruct, :undef)::BitVector | ||
| name === :undefs && return getfield(pstruct, :undefs)::Vector{Union{Nothing,Bool}} |
There was a problem hiding this comment.
NB: this method is not currently used from Compiler.jl. PR #57553 aims to fix that.
aviatesk
force-pushed
the
avi/strict-undef-fields
branch
from
b00838b to
e9ee6e4
Compare
This change allows `PartialStruct` to represent structs with strictly
uninitialized fields.
Now the previous `undef::BitVector` field is changed to
`undefs::Vector{Union{Nothing,Bool}}` to encode defined-ness information
of each field.
Also, this lets us fix the length of `typ::PartialStruct`'s `fields` to
always match the number of fields in `typ.typ`. Instead of the current
design where the length of `fields` changes depending on the number of
initialized fields, it seems simpler to have `PartialStruct`s
representing the same `typ` always have the same `fields` length.
So, I've included that refactoring as well.
- fixes the newly detected error in Oscar.jl
make `undefs::Vector{Union{Nothing,Bool}}`
aviatesk
force-pushed
the
avi/strict-undef-fields
branch
from
e9ee6e4 to
1475330
Compare
This change allows
PartialStructto represent structs with strictlyuninitialized fields.
Now the previous
undef::BitVectorfield is changed toundefs::Vector{Union{Nothing,Bool}}to encode defined-ness informationof each field.
Also, this lets us fix the length of
typ::PartialStruct'sfieldstoalways match the number of fields in
typ.typ. Instead of the currentdesign where the length of
fieldschanges depending on the number ofinitialized fields, it seems simpler to have
PartialStructsrepresenting the same
typalways have the samefieldslength.So, I've included that refactoring as well.