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

Missing TypedSlot in boolean if chains #36230

Closed
timholy opened this issue Jun 11, 2020 · 1 comment · Fixed by #36355
Closed

Missing TypedSlot in boolean if chains #36230

timholy opened this issue Jun 11, 2020 · 1 comment · Fixed by #36355
Assignees
Labels
compiler:inference Type inference compiler:latency Compiler latency

Comments

@timholy
Copy link
Member

timholy commented Jun 11, 2020

Inference fails to take full advantage of the knowledge that a and b are both Symbols:

julia> function symcmp(vec)
           a, b = vec[1], vec[2]
           if isa(a, Symbol) && isa(b, Symbol)
               return a == b
           end
           return false
       end
symcmp (generic function with 1 method)

julia> code_typed(symcmp, (Vector{Any},))
1-element Array{Any,1}:
 CodeInfo(
1 ── %1  = Base.arrayref(true, vec, 1)::Any%2  = Base.arrayref(true, vec, 2)::Any%3  = (%1 isa Main.Symbol)::Bool
└───       goto #3 if not %3
2 ── %5  = (%2 isa Main.Symbol)::Bool
└───       goto #4
3 ──       nothing::Nothing
4 ┄─ %8  = φ (#2 => %5, #3 => false)::Bool
└───       goto #11 if not %8
5 ── %10 = π (%2, Symbol)
│    %11 = Main.:(==)::Core.Compiler.Const(==, false)
│    %12 = (isa)(%1, WeakRef)::Bool
└───       goto #7 if not %12
6 ── %14 = π (%1, WeakRef)
│    %15 = invoke %11(%14::WeakRef, %10::Symbol)::Union{Missing, Bool}
└───       goto #10
7 ── %17 = (isa)(%1, Missing)::Bool
└───       goto #9 if not %17
8 ──       goto #10
9 ── %20 = (%1 == %10)::Union{Missing, Bool}
└───       goto #10
10%22 = φ (#6 => %15, #8 => $(QuoteNode(missing)), #9 => %20)::Union{Missing, Bool}
└───       return %22
11return false
) => Union{Missing, Bool}

Interestingly, the pure && chain does not have the same problem:

julia> function symcmp(vec)
           a, b = vec[1], vec[2]
           isa(a, Symbol) && isa(b, Symbol) && return a == b
           return false
       end
symcmp (generic function with 1 method)

julia> code_typed(symcmp, (Vector{Any},))
1-element Array{Any,1}:
 CodeInfo(
1%1 = Base.arrayref(true, vec, 1)::Any%2 = Base.arrayref(true, vec, 2)::Any%3 = (%1 isa Main.Symbol)::Bool
└──      goto #5 if not %3
2%5 = (%2 isa Main.Symbol)::Bool
└──      goto #4 if not %5
3%7 = π (%1, Symbol)
│   %8 = π (%2, Symbol)
│   %9 = (%7 === %8)::Bool
└──      return %9
4nothing::Nothing
5return false
) => Bool

It may not be exactly the same issue (it also involves constant-propagation), but this is also not inferred successfully:

julia> code_typed(Meta.isexpr, (Any, Symbol, Int))
1-element Array{Any,1}:
 CodeInfo(
1%1  = (ex isa Base.Meta.Expr)::Bool
└──       goto #3 if not %1
2%3  = π (ex, Expr)
│   %4  = Base.getfield(%3, :head)::Symbol%5  = (%4 === heads)::Bool
└──       goto #4
3 ─       goto #4
4%8  = φ (#2 => %5, #3 => false)::Bool
└──       goto #6 if not %8
5%10 = Base.getproperty(ex, :args)::Any%11 = Base.Meta.length(%10)::Any%12 = (%11 == n)::Any
└──       return %12
6return false
) => Any

And finally, in a dream world, inference would know that a successful isexpr call implies that ex::Expr. This is assumed all over the place, e.g.,

julia/base/show.jl

Lines 1038 to 1039 in 38f2c59

if kw && is_expr(item, :kw, 2)
show_unquoted(io, Expr(:(=), item.args[1], item.args[2]), indent, parens ? 0 : prec, quote_level)
to pick just one of many, but currently item.args is inferred as Any. This affects even the 2-argument form, e.g.,

julia> function useisexpr(ex)
           if isexpr(ex, :(=))
               lhs, rhs = ex.args[1], ex.args[2]
               return lhs
           end
           return nothing
       end

julia> code_typed(useisexpr, (Any,))
1-element Array{Any,1}:
 CodeInfo(
1%1  = (ex isa Base.Meta.Expr)::Bool
└──       goto #3 if not %1
2%3  = π (ex, Expr)
│   %4  = Base.getfield(%3, :head)::Symbol%5  = (%4 === :(=))::Bool
└──       goto #4
3 ─       goto #4
4%8  = φ (#2 => %5, #3 => false)::Bool
└──       goto #6 if not %8
5%10 = Base.getproperty(ex, :args)::Any%11 = Base.getindex(%10, 1)::Any%12 = Base.getproperty(ex, :args)::Any
│         Base.getindex(%12, 2)::Any
└──       return %11
6return Main.nothing
) => Any

I recognize this might be more than one issue and will be happy to split it up if desired.

I've tagged this with latency because it seems to be a really common source of invalidations for packages like JuliaInterpreter and similar that have to deal with a lot of poorly-typed data. I constantly think I'm being careful, but often it's not working. (Noticed while investigating #36005, though @KristofferC pointed this out to me earlier.)

@timholy timholy added the compiler:latency Compiler latency label Jun 11, 2020
@thofma
Copy link
Contributor

thofma commented Jun 11, 2020

Regarding the first observation, there was a comment by Jameson over at #35600 (comment):
"[...] inference currently can only propagate the last isa constraint on a line"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference compiler:latency Compiler latency
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants