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

Ambiguities in getindex introduced by Unrolled #9

Open
jishnub opened this issue Aug 1, 2020 · 6 comments
Open

Ambiguities in getindex introduced by Unrolled #9

jishnub opened this issue Aug 1, 2020 · 6 comments

Comments

@jishnub
Copy link

jishnub commented Aug 1, 2020

I'm on Julia 1.4.2 and using Unrolled version 0.1.3.

julia> using Test

julia> Test.detect_ambiguities(Base)
19-element Array{Tuple{Method,Method},1}:
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(io::IO, key) in Base at show.jl:332)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(d::IdDict{K,V}, key) where {K, V} in Base at abstractdict.jl:602)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(itr::Base.SkipMissing, I...) in Base at missing.jl:257)
 (getindex(md::Markdown.MD, args...) in Markdown at /home/jishnu/Downloads/julia-1.4.2/share/julia/stdlib/v1.4/Markdown/src/parse/parse.jl:24, getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23)
 (getindex(cache::LibGit2.CachedCredentials, cred_id) in LibGit2 at /home/jishnu/Downloads/julia-1.4.2/share/julia/stdlib/v1.4/LibGit2/src/types.jl:1275, getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(::Type{T}, x) where T in Base at array.jl:395)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(A::AbstractArray, I...) in Base at abstractarray.jl:978)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(t::AbstractDict, key) in Base at abstractdict.jl:469)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(::Type{Any}, vals...) in Base at array.jl:400)
 (getindex(r::Distributed.RemoteChannel, args...) in Distributed at /home/jishnu/Downloads/julia-1.4.2/share/julia/stdlib/v1.4/Distributed/src/remotecall.jl:637, getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(cmd::Cmd, i) in Base at process.jl:643)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(::Type{T}, vals...) where T in Base at array.jl:387)
 (getindex(t::REPL.Terminals.TTYTerminal, key) in REPL.Terminals at /home/jishnu/Downloads/julia-1.4.2/share/julia/stdlib/v1.4/REPL/src/Terminals.jl:175, getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(dict::Base.ImmutableDict, key) in Base at dict.jl:773)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(h::Dict{K,V}, key) where {K, V} in Base at dict.jl:476)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(wkh::WeakKeyDict{K,V} where V, key) where K in Base at weakkeydict.jl:111)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(io::IOContext, key) in Base at show.jl:331)
 (getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23, getindex(v::Base.Iterators.Pairs, key) in Base.Iterators at iterators.jl:250)
 (getindex(r::Distributed.Future, args...) in Distributed at /home/jishnu/Downloads/julia-1.4.2/share/julia/stdlib/v1.4/Distributed/src/remotecall.jl:635, getindex(seq, ::FixedRange{A,B}) where {A, B} in Unrolled at /home/jishnu/.julia/packages/Unrolled/26uDc/src/range.jl:23)
@cstjean
Copy link
Owner

cstjean commented Aug 2, 2020

Do you have a specific example that causes an issue? I didn't even document FixedRange, so I'm surprised someone is using it.

@jishnub
Copy link
Author

jishnub commented Aug 13, 2020

The issue is from a development point-of-view when a package uses Unrolled as a dependency. If package A uses Unrolled and tries to detect ambiguities by using Test.detect_ambiguities(Base, A), the result will be cluttered with ambiguities introduced by Unrolled that are unrelated to A. This is because of the ambiguities introduced in Base by Unrolled.

The ambiguities here are only triggered if someone actually uses FixedRange, however they make it inconvenient to debug downstream ambiguities.

@cstjean
Copy link
Owner

cstjean commented Aug 13, 2020

🤷 Feels like a pretty minor inconvenience. I'll be happy to merge a PR that fixes the issue, but it won't go on my todo list.

@cstjean
Copy link
Owner

cstjean commented Aug 13, 2020

Maybe that's a bug in detect_ambiguities? It should allow detect_ambiguities(MyPackage) which only show ambiguities that implicate a method defined in MyPackage.

@jishnub
Copy link
Author

jishnub commented Aug 13, 2020

It does indeed do that, but if a package is adding methods to Base then one needs to check that as well.

@jishnub jishnub closed this as completed Aug 13, 2020
@jishnub
Copy link
Author

jishnub commented Aug 13, 2020

Ah closed by mistake, sorry

@jishnub jishnub reopened this Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants