add constant inference of isdefined
#22304
Conversation
base/inference.jl
Outdated
| return Bool | ||
| end | ||
| if isleaftype(a1) | ||
| if a1 <: Array # TODO |
There was a problem hiding this comment.
This TODO can be removed now.
base/inference.jl
Outdated
| return Bool | ||
| end | ||
| if isleaftype(a1) | ||
| if a1 <: Array # TODO |
There was a problem hiding this comment.
this is deprecated now, should just remove this case along with the TODO
There was a problem hiding this comment.
I think we'll still need a case for this --- what will isdefined return on an Array when the deprecation is removed? Will it throw an error?
There was a problem hiding this comment.
Should be an error. Until the implementation of Array changes, in which case it might return a Bool again. In either case it should be a subtype of Bool.
base/inference.jl
Outdated
| if isType(a1) | ||
| return Bool | ||
| end | ||
| if isleaftype(a1) |
There was a problem hiding this comment.
This seems much stricter than necessary. I think isa(a1, DataType) && !a1.abstract should be sufficient
There was a problem hiding this comment.
Also can do a1 = unwrap_unionall(a1) first (before the isType) check as well.
base/inference.jl
Outdated
| end | ||
| if isleaftype(a1) | ||
| if a1 <: Array # TODO | ||
| elseif a1 === Module # TODO |
There was a problem hiding this comment.
I have the code for this case on https://github.com/JuliaLang/julia/pull/22281/files#diff-c0d983631c937ee5acfd0137cd7332deR2082. It's simple enough it's probably worth just including in this PR:
sym = args[2]
Symbol <: widenconst(sym) || return Bottom
if isa(sym, Const) && isa(sym.val, Symbol) && isdefined(a1, sym.val)
return Const(true)
end
base/inference.jl
Outdated
| end | ||
| Bool | ||
| end | ||
| add_tfunc(isdefined, 1, IInf, isdefined_tfunc) |
There was a problem hiding this comment.
upper end is actually 2, not IInf (noticed this on my PR https://github.com/JuliaLang/julia/pull/22281/files#diff-c0d983631c937ee5acfd0137cd7332deR545). (although when we remove the deprecations after v0.7, this'll just become constant at 2 args anyways)
base/inference.jl
Outdated
| if isa(val, Symbol) | ||
| idx = fieldindex(a1, val, false) | ||
| elseif isa(val, Int) | ||
| idx = val::Int |
There was a problem hiding this comment.
type assert is redundant with branch on isa Int and declaration of idx as being of type Int
test/inference.jl
Outdated
| @test Base.return_types(f, (Tuple{Int,Int},)) == Any[Int] | ||
| @test Base.return_types(f, (Tuple{Int,},)) == Any[String] | ||
| end | ||
| let f(x) = isdefined(x,:re) ? 1 : "" |
test/inference.jl
Outdated
| end | ||
| let f(x) = isdefined(x,:re) ? 1 : "" | ||
| @test Base.return_types(f, (Complex64,)) == Any[Int] | ||
| end |
There was a problem hiding this comment.
need test coverage for:
- isdefined with a symbol that isn't a valid field name
- isdefined with a non-symbol-or-integer
- isdefined with
xis an:SimpleVector,Module,Type,Array,Symbol - isdefined on a type where the field is not necessarily assigned
- isdefined on an abstract type
- isdefined on a UnionAll type
JeffBezanson
force-pushed
the
jb/isdefined_tfunc
branch
from
a979421 to
33cfb18
Compare
|
Comments addressed. |
| return Bool | ||
| end | ||
| a1 = unwrap_unionall(a1) | ||
| if isa(a1, DataType) && !a1.abstract |
There was a problem hiding this comment.
also need to verify that !isa(a1.parameters[end], Vararg)
There was a problem hiding this comment.
That's a good one --- that actually exposed a separate bug, fix added here.
JeffBezanson
force-pushed
the
jb/isdefined_tfunc
branch
from
33cfb18 to
ed999f2
Compare
JeffBezanson
force-pushed
the
jb/isdefined_tfunc
branch
2 times, most recently
from
9e1b5cc to
ca8b223
Compare
JeffBezanson
force-pushed
the
jb/isdefined_tfunc
branch
from
ca8b223 to
2d1d1ae
Compare
A bit taken from #16580.