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

Flawed dispatch triggered by changing (irrelevant) contents of a for loop over empty collection #36907

Closed
ablaom opened this issue Aug 4, 2020 · 2 comments
Assignees
Labels
types and dispatch Types, subtyping and method dispatch

Comments

@ablaom
Copy link

ablaom commented Aug 4, 2020

After investigating the source of JuliaAI/ScientificTypes.jl#104 I came up with the following MWE which, it appears to me, points to a Julia bug.

I've no idea why a SentinelArray triggers the weird dispatch and the other AbstractArray examples don't.

So here's the unexpected (I dare say wrong) behaviour.

using SentinelArrays, SparseArrays

abstract type IncorrectDispatch end

const dict = Dict{Symbol,Function}()

# as `dict` is empty the contents of the `for` block shouldn't matter:
function trait(X)::Symbol
    for (name, f) in dict
        if f(X)
            return name
        end
    end
    return :other
end

scitype(X) = scitype(X, Val(trait(X)))
scitype(X, ::Val{:other}) = IncorrectDispatch

# overload for AbstractArrays:
scitype(A::AbstractArray{T}, ::Val{:other}) where T =
    "correct dispatch"

# three different AbstractArrays with same elements:
a = [1, 2, 3, 4]
b = sparse(a)
c = SentinelArray(a)

# The `trait` function appears to work:
@assert trait(a) == :other
@assert trait(b) == :other
@assert trait(c) == :other

# these should all return  "correct dispatch" but the last one does not:
julia> scitype(a)
"correct dispatch"

julia> scitype(b)
"correct dispatch"

julia> scitype(c)
IncorrectDispatch

If, however, I change the contents of the for loop (which ought never to be executed) the wrong behaviour goes away:

function trait(X)::Symbol
    for (name, f) in dict
        if f(X)
            return :other
        end
    end
    return :other
end

julia> scitype(c)
"correct dispatch"
julia> Pkg.installed()
┌ Warning: Pkg.installed() is deprecated
└ @ Pkg /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.4/Pkg/src/Pkg.jl:531
Dict{String,VersionNumber} with 3 entries:
  "ScientificTypes"    => v"0.8.1"

julia> versioninfo()
Julia Version 1.4.2
Commit 44fa15b150* (2020-05-23 18:35 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin18.7.0)
  CPU: Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-8.0.1 (ORCJIT, skylake)
Environment:
  JULIA_PATH = /Applications/Julia-1.4.app/Contents/Resources/julia/bin/julia
  JULIA_NUM_THREADS = 5
@Keno
Copy link
Member

Keno commented Aug 4, 2020

SentinelArrays currently triggers some bad code paths in the type system (#36869). Probably worth waiting for that to be fixing before trying to reduce this much farther, since bad type system behavior can have all sorts of unexpected impacts.

@JeffBezanson
Copy link
Member

Confirmed fixed by #36996

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

No branches or pull requests

3 participants