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

Dispatching on T where Core.Compiler.iskindtype(T) is broken #11840

Closed
mauro3 opened this issue Jun 24, 2015 · 9 comments
Closed

Dispatching on T where Core.Compiler.iskindtype(T) is broken #11840

mauro3 opened this issue Jun 24, 2015 · 9 comments
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior types and dispatch Types, subtyping and method dispatch

Comments

@mauro3
Copy link
Contributor

mauro3 commented Jun 24, 2015

This testcase has a dispatch error in it:

# This just add debug info to Base.isbits(t::DataType) 
function Base.isbits(t::DataType)
    if !isa(t,DataType)
        @show typeof(t)
        error("Shouldn't dispatch to here!")
    end
    !t.mutable & t.pointerfree & isleaftype(t)
end
## # this fixes it:
## function Base.isbits(t::TypeConstructor)
##     false
## end

g() = Integer # needs to return a Type
gg(::Int) = nothing
tmm = first(methods(gg))
sig = Tuple{tmm.sig.parameters[2:end]...}
#    sig = (tmm.sig[2:end]...) # 0.3 version works
fret_typ = Base.return_types(g, sig)
println("$fret_typ")  # without this line no error is thrown later
@show methods(getindex) # error thrown here

This throws the error:

ERROR: LoadError: Shouldn't dispatch to here!

The indexing into the tuple is essential to trigger the bug.
The equivalent in 0.3 works.

(edit: slight update)

@tkelman tkelman added the types and dispatch Types, subtyping and method dispatch label Jun 24, 2015
@simonster simonster added bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version labels Aug 7, 2015
@simonster
Copy link
Member

This may be the same bug that is breaking examples/typetree.jl:

➜  julia git:(master) ✗ julia examples/typetree.jl 
+- Any << abstract immutable size:0 >>
.  +- => = Pair{A,B} << concrete immutable size:16 >>
.  +- AbstractArray = AbstractArray{T,2} << abstract immutable size:0 >>
.  .  +- AbstractArray{T,1} 
ERROR: LoadError: type TypeConstructor has no field mutable
 in isbits at reflection.jl:75
 in show_delim_array at ./show.jl:216
 in show at ./show.jl:73
 in print_to_string at strings/io.jl:19
 in print_tree at /usr/local/julia/examples/typetree.jl:103
 in print_tree at /usr/local/julia/examples/typetree.jl:108 (repeats 3 times)
 in print_tree at /usr/local/julia/examples/typetree.jl:101
 in include at ./boot.jl:254
 in include_from_node1 at ./loading.jl:263
 in process_options at ./client.jl:308
 in _start at ./client.jl:411
while loading /usr/local/julia/examples/typetree.jl, in expression starting on line 127

@simonster
Copy link
Member

Reduced test case:

julia> f(::Type) = "Type";

julia> f(::DataType) = "DataType";

julia> f(Type)
"DataType"

julia> f(AbstractVector)
"DataType"

If the order of the calls is reversed, then the result is correct:

julia> f(::Type) = "Type";

julia> f(::DataType) = "DataType";

julia> f(AbstractVector)
"Type"

julia> f(Type)
"DataType"

@joehuchette
Copy link
Member

I'm seeing the type TypeConstructor has no field mutable error, followed by a nice segfault:

ERROR: LoadError: LoadError: MethodError: `copy` has no method matching copy(::Array{JuMP.Variable,1}, ::JuMP.Model)
 in copy at /Users/huchette/.julia/v0.4/JuMP/src/JuMP.jl:258
 in anonymous at /Users/huchette/.julia/v0.4/JuMP/test/model.jl:354
 in facts at /Users/huchette/.julia/v0.4/FactCheck/src/FactCheck.jl:391
 in include at ./boot.jl:259
 in include_from_node1 at ./loading.jl:266
 in include at ./boot.jl:259
 in include_from_node1 at ./loading.jl:266
 in process_options at ./client.jl:308
 in _start at ./client.jl:411fatal: error thrown and no exception handler available.
ErrorException("type TypeConstructor has no field mutable")
rec_backtrace at /Users/huchette/julia/src/task.c:633
jl_vexceptionf at /Users/huchette/julia/usr/lib/libjulia.dylib (unknown line)
jl_errorf at /Users/huchette/julia/usr/lib/libjulia.dylib (unknown line)
jl_field_index at /Users/huchette/julia/usr/lib/libjulia.dylib (unknown line)
jl_f_get_field at /Users/huchette/julia/src/builtins.c:703
isbits at reflection.jl:75
jlcall_isbits_21994 at  (unknown line)
show_delim_array at show.jl:177
show_delim_array at show.jl:164
jlcall_show_delim_array_21991 at  (unknown line)
show at show.jl:74
show_type_parameter at ./show.jl:83
show at ./show.jl:93
jlcall_show_16978 at /Users/huchette/julia/usr/lib/julia/sys.dylib (unknown line)
show_type_parameter at ./show.jl:83
show at ./show.jl:93
jlcall_show_16978 at /Users/huchette/julia/usr/lib/julia/sys.dylib (unknown line)
show at expr.jl:56
show_type_parameter at ./show.jl:83
show at ./show.jl:93
jlcall_show_16978 at /Users/huchette/julia/usr/lib/julia/sys.dylib (unknown line)
print at strings/io.jl:5
print_to_string at ./strings/io.jl:19
string at strings/io.jl:26
show_method_candidates at replutil.jl:247
showerror at replutil.jl:191
showerror at replutil.jl:83
jlcall___showerror#156___27344 at  (unknown line)
jl_apply at /Users/huchette/julia/src/gf.c:1661
julia_showerror_27343 at  (unknown line)
julia_showerror_27342 at  (unknown line)
jlcall_showerror_27342 at  (unknown line)
showerror at replutil.jl:91
julia_showerror_27340 at  (unknown line)
jlcall_showerror_27340 at  (unknown line)
jl_apply at /Users/huchette/julia/src/gf.c:1661
anonymous at client.jl:88
with_output_color at util.jl:313
display_error at client.jl:86
jl_apply at /Users/huchette/julia/src/gf.c:1661
_start at ./client.jl:458
jlcall__start_18534 at /Users/huchette/julia/usr/lib/julia/sys.dylib (unknown line)
true_main at /Users/huchette/julia/julia (unknown line)
main at /Users/huchette/julia/julia (unknown line)

This is from the JuMP tests running in FactCheck; I haven't found a way to isolate the problem.

@vtjnash
Copy link
Member

vtjnash commented Aug 25, 2015

minimalist testcase:

julia> f(::DataType) = 1; f(x::Type) = 2
f (generic function with 2 methods)

julia> f(Vector.body) # this "poisons" the cache for when the identical TypeConstructor shows up later
1

julia> f(Vector) # this should return 2 since Vector is a TypeConstructor not a DataType
1

note that ::Type was necessary on at least one method to prevent

julia/src/gf.c

Line 639 in 8ea2ad7

jl_svecset(newparams, i, decl_i);
from de-specializing the cache method.

plus, we already missed out on some of the TypeConstructor "kind" cache fixup logic just above it because very_general_type(decl_i) returns false for DataType.

vtjnash added a commit that referenced this issue Aug 25, 2015
…he. fixes #11840

when TypeConstructors showed up as parameters of Type objects in the generic function cache,
the type intersection result would trick inference into caching it in the wrong place
and using it when it didn't apply. fix that by checking explicitly for arguments declared
with ::TypeConstructor and ensuring they are matched and cached correctly
vtjnash added a commit that referenced this issue Aug 25, 2015
…or inserting/matching Type{T} in the gf cache
mauro3 added a commit to mauro3/Traits.jl that referenced this issue Aug 27, 2015
Fix of JuliaLang/julia#11840 incorporated.

Julia regression uncovered in tests:
JuliaLang/julia#12826
vtjnash added a commit that referenced this issue Apr 3, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches
this also improves the fix for #11840 to cover Union
but breaks the fix for #11355 -- fortunately, our improved cache should soon be up to the challenge of fixing it too
vtjnash added a commit that referenced this issue Apr 5, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 5, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 5, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 6, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 6, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit to vtjnash/julia that referenced this issue Apr 7, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for JuliaLang#11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 9, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 11, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 12, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 12, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 12, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 13, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 15, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
vtjnash added a commit that referenced this issue Apr 15, 2016
this allows us to correct the type signature before passing it to the generated function
or returning it from ml_matches

this also improves the fix for #11840 to cover Unions of the offending types
@vtjnash vtjnash self-assigned this Dec 31, 2016
@vtjnash vtjnash reopened this Dec 31, 2016
@vtjnash
Copy link
Member

vtjnash commented Dec 31, 2016

The test for this is broken and thus disabled. I think I know a fix for it.

@JeffBezanson
Copy link
Member

closed by #18457 (I think? Might have missed something here.)

mauro3 added a commit to mauro3/julia that referenced this issue Jan 17, 2017
mauro3 added a commit to mauro3/julia that referenced this issue Jan 17, 2017
mauro3 added a commit to mauro3/julia that referenced this issue Jan 17, 2017
mauro3 added a commit to mauro3/julia that referenced this issue Jan 17, 2017
@vtjnash vtjnash reopened this Jan 17, 2017
@vtjnash
Copy link
Member

vtjnash commented Jan 17, 2017

no, this isn't a subtyping bug.

mauro3 added a commit to mauro3/julia that referenced this issue Jan 18, 2017
Added tests for issues JuliaLang#12580, JuliaLang#18348, JuliaLang#13165, JuliaLang#11803, JuliaLang#12721

Enabled extra tests for JuliaLang#11840, however, that isssue is not resolved
yet but needs tests triggering it.
mauro3 added a commit to mauro3/julia that referenced this issue Jan 20, 2017
Added tests for issues JuliaLang#12580, JuliaLang#18348, JuliaLang#13165, JuliaLang#12721

For JuliaLang#11803 it was decidided that no tests are needed.

Enabled extra tests for JuliaLang#11840, however, that isssue is not resolved
yet but needs new tests triggering it.
mauro3 added a commit to mauro3/julia that referenced this issue Jan 20, 2017
Added tests for issues JuliaLang#12580, JuliaLang#18348, JuliaLang#13165, JuliaLang#12721

For JuliaLang#11803 it was decidided that no tests are needed.

Enabled extra tests for JuliaLang#11840, however, that isssue is not resolved
yet but needs new tests triggering it.
@KristofferC
Copy link
Member

Code in OP works fine now.

@tkelman
Copy link
Contributor

tkelman commented May 25, 2017

no, this isn't a subtyping bug.

What is it then?

@vtjnash vtjnash reopened this Jul 13, 2017
@jrevels jrevels changed the title Dispatch error Dispatching on T where Core.Compiler.iskindtype(T) is broken Apr 30, 2018
@jrevels jrevels mentioned this issue Apr 30, 2018
5 tasks
@vtjnash vtjnash removed the regression Regression in behavior compared to a previous version label May 4, 2018
vtjnash added a commit that referenced this issue May 4, 2018
respect type-equality, keep track separately of what Kind it needs to match
revert dfd8fc1
fix #11840 (except for generated functions)
vtjnash added a commit that referenced this issue May 10, 2018
respect type-equality, keep track separately of what Kind it needs to match
revert dfd8fc1
fix #11840 (except for generated functions)
vtjnash added a commit that referenced this issue May 10, 2018
respect type-equality, keep track separately of what Kind it needs to match
revert dfd8fc1
fix #11840 (except for generated functions)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

No branches or pull requests

7 participants