fix erroneous code path within `show_sym` #38830

aviatesk · 2020-12-11T09:58:44Z

currently, show_sym can recur into show(::IO, String), but it's a
bit "erroneous" since is_syntactic_operator(::String) is not defined.

the virtual stacktrace from JET.jl
may describe this better:

julia> @report_call println(QuoteNode(nothing))
═════ 1 possible error found ═════
┌ @ coreio.jl:4 Base.println(Core.tuple(Core.typeassert(Base.stdout, Base.IO)), xs...)
│┌ @ strings/io.jl:73 Base.print(Core.tuple(io), xs, Core.tuple("\n")...)
││┌ @ strings/io.jl:46 Base.print(io, x)
│││┌ @ show.jl:1144 Base.show_unquoted(Base.IOContext(io, Base.=>(:unquote_fallback, false)), ex, 0, -1)
││││┌ @ show.jl:1496 Base.show_unquoted_quote_expr(io, Base.getproperty(ex, :value), indent, prec, 0)
│││││┌ @ show.jl:1522 Base.show_block(Base.IOContext(io, Base.=>(Base.beginsym, false)), "quote", value, indent,quote_level)
││││││┌ @ show.jl:1365 Base.show_block(io, head, Base.vect(), block, i, quote_level)
│││││││┌ @ show.jl:1361 Base.show_unquoted(io, ex, ind, -1, quote_level)
││││││││┌ @ show.jl:1955 Core.kwfunc(Base.show_globalref)(Core.apply_type(Core.NamedTuple, (:allow_macroname, (Core.tuple(true)), Base.show_globalref, io, arg1)
│││││││││┌ @ show.jl:1468 Base.#show_globalref#424(allow_macroname, _3, io, ex)
││││││││││┌ @ show.jl:1474 Core.kwfunc(Base.show_sym)(Core.apply_type(Core.NamedTuple, (:allow_macroname,)(Core.tuple(allow_macroname)), Base.show_sym, io, Base.getproperty(ex, :name))
│││││││││││┌ @ REPL[7]:2 Base.#show_sym#882(allow_macroname, _3, io, sym)
││││││││││││┌ @ REPL[7]:6 Base.show_sym(io, Base.getindex(sym_str, Base.:(2, Base.lastindex(sym_str))))
│││││││││││││┌ @ REPL[7]:2 Base.#show_sym#882(false, #self#, io, sym)
││││││││││││││┌ @ REPL[7]:2 Base.is_valid_identifier(sym)
│││││││││││││││┌ @ show.jl:1410 Base.is_syntactic_operator(sym)
││││││││││││││││ no matching method found for call signature: Base.is_syntactic_operator(sym::String)
│││││││││││││││└────────────────
Core.Const(nothing)

julia> # fix this
       @eval Base function show_sym(io::IO, sym; allow_macroname=false)
           if is_valid_identifier(sym)
               print(io, sym)
           elseif allow_macroname && (sym_str = string(sym); 
startswith(sym_str, '@'))
               print(io, '@')
               show_sym(io, Symbol(sym_str[2:end]))
           else
               print(io, "var", repr(string(sym)))
           end
       end
show_sym (generic function with 1 method)

julia> @report_call println(QuoteNode(nothing))
No errors !
Core.Const(nothing)

@eval

currently, `show_sym` can recur into `show(::IO, String)`, but it's a bit "erroneous" since `is_syntactic_operator(::String)` is not defined. the virtual stacktrace from [JET.jl](https://github.com/aviatesk/JET.jl) may describe this better: ```julia julia> @report_call println(QuoteNode(nothing)) ═════ 1 possible error found ═════ ┌ @ coreio.jl:4 Base.println(Core.tuple(Core.typeassert(Base.stdout, Base.IO)), xs...) │┌ @ strings/io.jl:73 Base.print(Core.tuple(io), xs, Core.tuple("\n")...) ││┌ @ strings/io.jl:46 Base.print(io, x) │││┌ @ show.jl:1144 Base.show_unquoted(Base.IOContext(io, Base.=>(:unquote_fallback, false)), ex, 0, -1) ││││┌ @ show.jl:1496 Base.show_unquoted_quote_expr(io, Base.getproperty(ex, :value), indent, prec, 0) │││││┌ @ show.jl:1522 Base.show_block(Base.IOContext(io, Base.=>(Base.beginsym, false)), "quote", value, indent, quote_level) ││││││┌ @ show.jl:1365 Base.show_block(io, head, Base.vect(), block, i, quote_level) │││││││┌ @ show.jl:1361 Base.show_unquoted(io, ex, ind, -1, quote_level) ││││││││┌ @ show.jl:1955 Core.kwfunc(Base.show_globalref)(Core.apply_type(Core.NamedTuple, (:allow_macroname,))(Core.tuple(true)), Base.show_globalref, io, arg1) │││││││││┌ @ show.jl:1468 Base.#show_globalref#424(allow_macroname, _3, io, ex) ││││││││││┌ @ show.jl:1474 Core.kwfunc(Base.show_sym)(Core.apply_type(Core.NamedTuple, (:allow_macroname,))(Core.tuple(allow_macroname)), Base.show_sym, io, Base.getproperty(ex, :name)) │││││││││││┌ @ REPL[7]:2 Base.#show_sym#882(allow_macroname, _3, io, sym) ││││││││││││┌ @ REPL[7]:6 Base.show_sym(io, Base.getindex(sym_str, Base.:(2, Base.lastindex(sym_str)))) │││││││││││││┌ @ REPL[7]:2 Base.#show_sym#882(false, #self#, io, sym) ││││││││││││││┌ @ REPL[7]:2 Base.is_valid_identifier(sym) │││││││││││││││┌ @ show.jl:1410 Base.is_syntactic_operator(sym) ││││││││││││││││ no matching method found for call signature: Base.is_syntactic_operator(sym::String) │││││││││││││││└──────────────── Core.Const(nothing) julia> # fix this @eval Base function show_sym(io::IO, sym; allow_macroname=false) if is_valid_identifier(sym) print(io, sym) elseif allow_macroname && (sym_str = string(sym); startswith(sym_str, '@')) print(io, '@') show_sym(io, Symbol(sym_str[2:end])) else print(io, "var", repr(string(sym))) end end show_sym (generic function with 1 method) julia> @report_call println(QuoteNode(nothing)) No errors ! Core.Const(nothing) ```

JeffBezanson · 2020-12-11T15:38:36Z

Let's also add sym::Symbol to the argument.

this makes it test pass regardless of JuliaLang/julia#38830

vchuravy · 2020-12-14T17:58:44Z

@JeffBezanson should this be backported to 1.6?

fix erroneous code path within `show_sym` currently, `show_sym` can recur into `show(::IO, String)`, but it's a bit "erroneous" since `is_syntactic_operator(::String)` is not defined.

antimon2 · 2020-12-23T02:55:30Z

I have sent PR to backports-release-1.6 ( #38955 ) due to the following error in Julia v1.6-DEV:

julia-1.6.0-DEV.1722> :(@..{1,2,..})
:(#= REPL[1]:1 =# @Error showing value of type Expr:
ERROR: MethodError: no method matching is_syntactic_operator(::String)
Closest candidates are:
  is_syntactic_operator(::Symbol) at show.jl:1221
Stacktrace:
  [1] is_valid_identifier
    @ ./show.jl:1399 [inlined]
  [2] show_sym(io::IOContext{Base.TTY}, sym::String; allow_macroname::Bool)
    @ Base ./show.jl:1438
  [3] show_sym
    @ ./show.jl:1438 [inlined]
  [4] show_sym(io::IOContext{Base.TTY}, sym::Symbol; allow_macroname::Bool)
    @ Base ./show.jl:1442
  [5] show_unquoted(io::IOContext{Base.TTY}, ex::Expr, indent::Int64, prec::Int64, quote_level::Int64)
    @ Base ./show.jl:1942
  [6] show_list(io::IOContext{Base.TTY}, items::Vector{Any}, sep::Char, indent::Int64, prec::Int64, quote_level::Int64, enclose_operators::Bool, kw::Bool)
    @ Base ./show.jl:1385
  [7] show_list
    @ ./show.jl:1366 [inlined]
  [8] show_unquoted(io::IOContext{Base.TTY}, ex::Expr, indent::Int64, prec::Int64, quote_level::Int64)
    @ Base ./show.jl:1935
  [9] show_unquoted_quote_expr(io::IOContext{Base.TTY}, value::Any, indent::Int64, prec::Int64, quote_level::Int64)
    @ Base ./show.jl:1515
 [10] show
    @ ./show.jl:1134 [inlined]
 [11] show(io::IOContext{Base.TTY}, #unused#::MIME{Symbol("text/plain")}, x::Expr)
    @ Base.Multimedia ./multimedia.jl:47
 [12] (::REPL.var"#38#39"{REPL.REPLDisplay{REPL.LineEditREPL}, MIME{Symbol("text/plain")}, Base.RefValue{Any}})(io::Any)
    @ REPL /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/REPL.jl:220
 [13] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
    @ REPL /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/REPL.jl:462
 [14] display(d::REPL.REPLDisplay, mime::MIME{Symbol("text/plain")}, x::Any)
    @ REPL /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/REPL.jl:213
 [15] display(d::REPL.REPLDisplay, x::Any)
    @ REPL /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/REPL.jl:225
 [16] display(x::Any)
    @ Base.Multimedia ./multimedia.jl:328
 [17] #invokelatest#2
    @ ./essentials.jl:707 [inlined]
 [18] invokelatest
    @ ./essentials.jl:706 [inlined]
 [19] print_response(errio::IO, response::Any, show_value::Bool, have_color::Bool, specialdisplay::Union{Nothing, AbstractDisplay})
    @ REPL /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/REPL.jl:247
 [20] (::REPL.var"#40#41"{REPL.LineEditREPL, Pair{Any, Bool}, Bool, Bool})(io::Any)
    @ REPL /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/REPL.jl:231
 [21] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
    @ REPL /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/REPL.jl:462
 [22] print_response(repl::REPL.AbstractREPL, response::Any, show_value::Bool, have_color::Bool)
    @ REPL /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/REPL.jl:229
 [23] (::REPL.var"#do_respond#61"{Bool, Bool, REPL.var"#72#82"{REPL.LineEditREPL, REPL.REPLHistoryProvider}, REPL.LineEditREPL, REPL.LineEdit.Prompt})(s::REPL.LineEdit.MIState, buf::Any, ok::Bool)
    @ REPL /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/REPL.jl:798
 [24] #invokelatest#2
    @ ./essentials.jl:707 [inlined]
 [25] invokelatest
    @ ./essentials.jl:706 [inlined]
 [26] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/LineEdit.jl:2441
 [27] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
    @ REPL /mnt/polonium_data/github/JuliaLang/julia/usr/share/julia/stdlib/v1.6/REPL/src/REPL.jl:1126
 [28] (::REPL.var"#44#49"{REPL.LineEditREPL, REPL.REPLBackendRef})()
    @ REPL ./task.jl:406

This error will be fixed by applying this commit.
(↓ in my local)

julia-1.6.0-DEV.1780> :(@..{1,2,..})
:(#= REPL[1]:1 =# @.. {1, 2, ..})

fix erroneous code path within `show_sym` currently, `show_sym` can recur into `show(::IO, String)`, but it's a bit "erroneous" since `is_syntactic_operator(::String)` is not defined. (cherry picked from commit 6322b8c)

fix erroneous code path within `show_sym` currently, `show_sym` can recur into `show(::IO, String)`, but it's a bit "erroneous" since `is_syntactic_operator(::String)` is not defined.

fix erroneous code path within `show_sym` currently, `show_sym` can recur into `show(::IO, String)`, but it's a bit "erroneous" since `is_syntactic_operator(::String)` is not defined. (cherry picked from commit 6322b8c)

aviatesk requested a review from c42f December 11, 2020 09:58

explicit argument type for show_sym

f13c310

JeffBezanson approved these changes Dec 11, 2020

View reviewed changes

aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Dec 12, 2020

more robust testing for show_sym invalidation

9d6057e

this makes it test pass regardless of JuliaLang/julia#38830

JeffBezanson merged commit 6322b8c into JuliaLang:master Dec 14, 2020

aviatesk deleted the show_sym branch December 15, 2020 05:44

antimon2 mentioned this pull request Dec 21, 2020

Cherry-Picked: fix erroneous code path within show_sym (#38830) #38955

Closed

antimon2 mentioned this pull request Jan 9, 2021

MethodError: no method matching is_syntactic_operator #39156

Closed

simeonschaub added the backport 1.6 Change should be backported to release-1.6 label Jan 9, 2021

simeonschaub added this to the 1.6 blockers milestone Jan 9, 2021

KristofferC mentioned this pull request Jan 19, 2021

Backports 1.6-rc1 #39160

Merged

60 tasks

KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Feb 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix erroneous code path within `show_sym` #38830

fix erroneous code path within `show_sym` #38830

aviatesk commented Dec 11, 2020 •

edited

Loading

JeffBezanson commented Dec 11, 2020

vchuravy commented Dec 14, 2020

antimon2 commented Dec 23, 2020 •

edited

Loading

fix erroneous code path within show_sym #38830

fix erroneous code path within show_sym #38830

Conversation

aviatesk commented Dec 11, 2020 • edited Loading

JeffBezanson commented Dec 11, 2020

vchuravy commented Dec 14, 2020

antimon2 commented Dec 23, 2020 • edited Loading

fix erroneous code path within `show_sym` #38830

fix erroneous code path within `show_sym` #38830

aviatesk commented Dec 11, 2020 •

edited

Loading

antimon2 commented Dec 23, 2020 •

edited

Loading