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

Internal error on "showing value of type Base.MethodList" #24195

Closed
traktofon opened this issue Oct 18, 2017 · 22 comments
Closed

Internal error on "showing value of type Base.MethodList" #24195

traktofon opened this issue Oct 18, 2017 · 22 comments
Labels
types and dispatch Types, subtyping and method dispatch

Comments

@traktofon
Copy link
Contributor

Hi,

while trying to prepare my package for julia-0.7, I encountered a situation where calling an ambiguous method leads to an internal error. Even fixing the ambiguity, I get an internal error when calling methods on the function in question. Unfortunately I haven't been able to produce a minimal example, as I can't trigger the internal error when substantially removing methods from the function.

This session shows how the bug can be triggered:

julia> versioninfo()
Julia Version 0.7.0-DEV.2197
Commit baa0879* (2017-10-18 10:29 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
  WORD_SIZE: 64
  BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY Haswell)
  LAPACK: libopenblas64_
  LIBM: libopenlibm
  LLVM: libLLVM-3.9.1 (ORCJIT, skylake)
Environment:

julia> Pkg.add("FortranFiles")
INFO: Installing FortranFiles v0.3.0
INFO: Package database updated

julia> Pkg.checkout("FortranFiles")
INFO: Checking out FortranFiles master...
INFO: Pulling FortranFiles latest master...
INFO: No packages to install, update or remove

julia> using FortranFiles

julia> methods(FortranFiles.write_var)
# 12 methods for generic function "write_var":
[1] write_var(rec::FortranFiles.Record, var::Int8) in FortranFiles at /home/frank/.julia/v0.7/FortranFiles/src/write.jl:45
[2] write_var(rec::FortranFiles.RecordWithSubrecords{FortranFiles.Conversion{typeof(identity),typeof(identity)}}, arr::Array{Int8,N}) where N in FortranFiles at /home/frank/.julia/v0.7/FortranFiles/src/write.jl:74
[3] write_var(rec::FortranFiles.RecordWithoutSubrecords{R,FortranFiles.Conversion{typeof(identity),typeof(identity)}}, arr::Array{Int8,N}) where {N, R} in FortranFiles at /home/frank/.julia/v0.7/FortranFiles/src/write.jl:76
[4] write_var(rec::FortranFiles.Record, arr::Array{Int8,N}) where N in FortranFiles at /home/frank/.julia/v0.7/FortranFiles/src/write.jl:49
[5] Error showing value of type Base.MethodList:
ERROR: UndefVarError: N not defined
Stacktrace:
 [1] print(::IOContext{Base.GenericIOBuffer{Array{UInt8,1}}}, ::Type{SYSTEM: show(lasterr) caused an error
UndefVarError(:N)

Stacktrace:
 [1] print(::IOContext{Base.GenericIOBuffer{Array{UInt8,1}}}, ::Type{ERROR: UndefVarError: N not defined
Stacktrace:
 [1] print(::IOContext{Base.Terminals.TTYTerminal}, ::Type{fatal: error thrown and no exception handler available.
UndefVarError(var=:N)
rec_backtrace at /home/frank/src/julia/src/stackwalk.c:87
record_backtrace at /home/frank/src/julia/src/task.c:246
jl_throw at /home/frank/src/julia/src/task.c:577
jl_undefined_var_error at /home/frank/src/julia/src/rtutils.c:129
print at /home/frank/.julia/v0.7/FortranFiles/src/string.jl:21
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:370 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
show at /home/frank/.julia/v0.7/FortranFiles/src/string.jl:22
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:370 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
show_datatype at ./show.jl:311
show at ./show.jl:242
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
print at ./strings/io.jl:30
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
print at ./strings/io.jl:41
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
show_tuple_as_call at ./show.jl:1257
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
show_spec_linfo at ./stacktraces.jl:221
#show#9 at ./stacktraces.jl:229
unknown function (ip: 0x7ffae570bcb5)
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
jl_apply at /home/frank/src/julia/src/julia.h:1449 [inlined]
jl_invoke at /home/frank/src/julia/src/gf.c:51
#show at ./<missing>:0
#show_trace_entry#528 at ./replutil.jl:628
unknown function (ip: 0x7ffae570b191)
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
#show_trace_entry at ./<missing>:0
#530 at ./replutil.jl:645
unknown function (ip: 0x7ffae570abcc)
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
#process_backtrace#533 at ./replutil.jl:676
unknown function (ip: 0x7ffae570a77c)
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
jl_apply at /home/frank/src/julia/src/julia.h:1449 [inlined]
jl_invoke at /home/frank/src/julia/src/gf.c:51
show_backtrace at ./replutil.jl:648
#showerror#510 at ./replutil.jl:226
unknown function (ip: 0x7ffae5709359)
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
jl_apply at /home/frank/src/julia/src/julia.h:1449 [inlined]
jl_invoke at /home/frank/src/julia/src/gf.c:51
showerror at ./replutil.jl:221
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
display_error at ./client.jl:139
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
display_error at ./client.jl:142
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
do_call at /home/frank/src/julia/src/interpreter.c:70
eval at /home/frank/src/julia/src/interpreter.c:262
eval_body at /home/frank/src/julia/src/interpreter.c:552
jl_toplevel_eval_body at /home/frank/src/julia/src/interpreter.c:524
jl_toplevel_eval_flex at /home/frank/src/julia/src/toplevel.c:634
jl_toplevel_eval_in at /home/frank/src/julia/src/builtins.c:626
eval at ./sysimg.jl:53
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
_start at ./client.jl:443
jl_call_fptr_internal at /home/frank/src/julia/src/julia_internal.h:366 [inlined]
jl_call_method_internal at /home/frank/src/julia/src/julia_internal.h:385 [inlined]
jl_apply_generic at /home/frank/src/julia/src/gf.c:2003
jl_apply at /home/frank/src/julia/ui/../src/julia.h:1449 [inlined]
true_main at /home/frank/src/julia/ui/repl.c:107
main at /home/frank/src/julia/ui/repl.c:237
__libc_start_main at /build/glibc-MECilU/glibc-2.24/csu/../csu/libc-start.c:291
_start at ./julia (unknown line)

v0.6 of julia doesn't exhibit this behavior, it just shows all the methods.

Thanks.

@JeffBezanson
Copy link
Member

While I believe we can fix this on our end (note to self: seems to be due to dispatch on types containing free variables), please do not define methods like these:

print(io::IO, ::Type{FString{N}}) where {N} = print(io, "FString{$N}")
show(io::IO, T::Type{FString{N}}) where {N} = print(io, T)

This redefines how types get printed, which can interfere with many things in the system such as printing method tables. While these particular definitions are very simple, if they were more complicated and had bugs, you (and other users of the package) could end up in a situation where you can't debug anything since nothing can be printed. The term "type piracy" gets thrown around, but I've taken to calling this kind of case "type treason".

@JeffBezanson JeffBezanson added the types and dispatch Types, subtyping and method dispatch label Oct 18, 2017
@traktofon
Copy link
Contributor Author

Hi Jeff,
thanks for looking into this. To make sure I understand correctly: I shouldn't override how types get printed, but it's still ok to override how instances get printed/shown, right? I believe I want to achieve the latter, and the two methods you quoted are leftovers from my attempts of doing so. I'll follow your advice and remove those. Thanks.

@JeffBezanson
Copy link
Member

Yes, correct.

@stevengj
Copy link
Member

Note also that you should almost never override print. It is sufficient to override show, since print calls show. See https://docs.julialang.org/en/latest/manual/types/#Custom-pretty-printing-1

@traktofon
Copy link
Contributor Author

Update: after removing the print and show methods for the type, the issue no longer appears. Indeed, the item in the methods output where the error occured does involve showing the FString type.

While I still feel that user code should not cause internal errors, it would be okay for me to close this.

@helgee
Copy link
Contributor

helgee commented Nov 16, 2017

I recently ran into a similar problem. If overloading show for types is considered harmful, would it be possible to prevent the user from the doing that? If not, this should be documented and I will open a documentation PR.

@iamed2
Copy link
Contributor

iamed2 commented May 15, 2018

Is it okay to define show methods on a type for a specific MIME-type? How about display?

@JeffBezanson
Copy link
Member

I believe only providers of displays should define display, not the objects being displayed.

I guess it's slightly less bad to define a show method for ::Type{T} for a non-text/plain MIME type.

@iamed2
Copy link
Contributor

iamed2 commented May 15, 2018

I was hoping for test/plain actually. Mainly I want to change how a type prints at the REPL, at the minimum.

@JeffBezanson
Copy link
Member

Example?

@JeffBezanson
Copy link
Member

Yes, I guess defining show for text/plain is also slightly less bad than defining 2-arg show or print or string.

@iamed2
Copy link
Contributor

iamed2 commented May 15, 2018

invenia/Intervals.jl#31

Basically we want a type alias to show up as its alias rather than the underlying UnionAll.

@vtjnash
Copy link
Member

vtjnash commented May 15, 2018

This is now supported:

help?> Base.showarg
  showarg(io::IO, x, toplevel)

  Show x as if it were an argument to a function. This function is used by summary to display type information in terms of sequences of function calls on objects. toplevel is true if this is
  the direct call from summary and false for nested (recursive) calls.

@ararslan
Copy link
Member

showarg isn't the same thing; it doesn't change how something is printed aside from in summary.

@iamed2
Copy link
Contributor

iamed2 commented May 16, 2018

It will probably help mitigate the downside though, thanks

@iamed2
Copy link
Contributor

iamed2 commented May 16, 2018

Never mind, it doesn't apply here. showarg is for when you control a collection type. We want to change how the type appears as a parameter to other types.

Why does Base need show(io::IO, ::Type) to behave a certain way? Why not have sensitive code like method table printing use an inner showtype function and allow users to override show for their specific types?

@omus
Copy link
Member

omus commented Aug 18, 2021

Yes, I guess defining show for text/plain is also slightly less bad than defining 2-arg show or print or string.

@JeffBezanson could you clarify this statement? Should we be avoiding defining methods such as Base.show(::IO, ::MIME"text/plain", ::Type{MyType})?

More specifically I'm looking into making a type similar to MIME:

julia> struct TZ{name} end

julia> macro TZ_str(name)
           :(TZ{$(Expr(:quote, Symbol(name)))})
       end
@TZ_str (macro with 1 method)

julia> TZ"A/B"
TZ{Symbol("A/B")}

I'd like to avoid the extra verbosity present when using Symbols containing "/" by showing the string macro instead. That can be accomplished easily enough with overloading show:

julia> Base.show(io::IO, m::MIME"text/plain", T::Type{TZ}) = print(io, "TZ")

julia> function Base.show(io::IO, m::MIME"text/plain", T::Type{<:TZ})
           name = T.parameters[1]
           if name isa Symbol
               print(io, "TZ\"$name\"")
           else
               invoke(show, Tuple{IO, MIME"text/plain", Type}, io, m, T)
           end
       end

julia> TZ"A/B"
TZ"A/B"

julia> TZ
TZ

julia> TZ{1}
TZ{1}

However, the main use for this is as a type parameter for another type. That means that using MIME"text/plain" isn't going to work:

julia> struct ZDT{T <: TZ} end

julia> ZDT{TZ"A/B"}
ZDT{TZ{Symbol("A/B")}}

Looking into the code Base.show(::IO, ::Type) calls Base.show_datatype which eventually calls Base.show(::IO, ::Type) for the type parameters. This leads me back to wanting to adding methods such as Base.show(::IO, ::Type{<:TZ}):

julia> Base.show(io::IO, T::Type{TZ}) = print(io, "TZ")

julia> function Base.show(io::IO, T::Type{<:TZ})
           name = T.parameters[1]
           if name isa Symbol
               print(io, "TZ\"$name\"")
           else
               invoke(show, Tuple{IO, Type}, io, T)
           end
       end

julia> ZDT{TZ"A/B"}
ZDT{TZ"A/B"}

julia> ZDT{TZ}
ZDT{TZ}

julia> ZDT{TZ{1}}
ZDT{TZ{1}}

So far this implementation seems to work great but it's definitely very easy to mess up method printing if you aren't careful in how you define these methods.

Is there a path forward on custom show methods for user defined types? Should we add official support for this via a new function (e.g. show_type)? Or some other approach? I'd really like to not have the take away from this issue be "don't do this".

@vtjnash
Copy link
Member

vtjnash commented Aug 18, 2021

If you add an incorrect definition for show on a Type, such as the one you posted there, things will break: don't do that!

Closing, as I am not seeing too much actionable in this issue right now. In particular, we may want to fix the issue where we may dispatch an improper type as if it was a subtype of a proper type, but I don't think this issue reflects that in a way which would enable us to find this issue when we fix that.

@vtjnash vtjnash closed this as completed Aug 18, 2021
@iamed2
Copy link
Contributor

iamed2 commented Aug 18, 2021

@vtjnash my main questions was not answered:

Why does Base need show(io::IO, ::Type) to behave a certain way? Why not have sensitive code like method table printing use an inner showtype function and allow users to override show for their specific types?

I understand the impact given the current behaviour of Base, but I cannot see a reason why the current behaviour is necessary. Is show simply buried so far down in the call stack under e.g. showing a method table that it cannot be replaced? Could Base use a world-age-frozen version of show for critical functions?

@vtjnash
Copy link
Member

vtjnash commented Aug 18, 2021

What is the purpose of defining show logic for a Type, then telling users that they must actually call working_show to bypass it?

@omus omus added the triage This should be discussed on a triage call label Aug 19, 2021
@omus
Copy link
Member

omus commented Aug 19, 2021

I've added the triage label as I think a quick conversation could clear some things up.

@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Feb 17, 2022
@JeffBezanson
Copy link
Member

From triage: all we can really do is recommend that people not define this, or be very careful if they do. show is obviously going to be called in many contexts, and if your show method errors that's what you get.

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

8 participants