improve display of keyword argument methods in stack traces (#33118)

part of #33065

Author: JeffBezanson

base/errorshow.jl

@@ -181,7 +181,7 @@ function showerror(io::IO, ex::MethodError)
     name = ft.name.mt.name
     f_is_function = false
     kwargs = ()
-    if startswith(string(ft.name.name), "#kw#")
+    if endswith(string(ft.name.name), "##kw")
         f = ex.args[2]
         ft = typeof(f)
         name = ft.name.mt.name
@@ -416,11 +416,10 @@ function show_method_candidates(io::IO, ex::MethodError, @nospecialize kwargs=()
                         end
                     end
                 end
-                kwords = Symbol[]
-                if isdefined(ft.name.mt, :kwsorter)
-                    kwsorter_t = typeof(ft.name.mt.kwsorter)
-                    kwords = kwarg_decl(method, kwsorter_t)
-                    length(kwords) > 0 && print(iob, "; ", join(kwords, ", "))
+                kwords = kwarg_decl(method)
+                if !isempty(kwords)
+                    print(iob, "; ")
+                    join(iob, kwords, ", ")
                 end
                 print(iob, ")")
                 show_method_params(iob0, tv)
@@ -602,6 +601,11 @@ function show_backtrace(io::IO, t::Vector{Any})
     end
 end
 
+function is_kw_sorter_name(name::Symbol)
+    sn = string(name)
+    return !startswith(sn, '#') && endswith(sn, "##kw")
+end
+
 function process_backtrace(t::Vector, limit::Int=typemax(Int); skipC = true)
     n = 0
     last_frame = StackTraces.UNKNOWN
@@ -621,7 +625,7 @@ function process_backtrace(t::Vector, limit::Int=typemax(Int); skipC = true)
                 continue
             end
 
-            if lkup.from_c && skipC
+            if (lkup.from_c && skipC) || is_kw_sorter_name(lkup.func)
                 continue
             end
             count += 1

base/methodshow.jl

@@ -73,23 +73,27 @@ end
 
 const empty_sym = Symbol("")
 
-function kwarg_decl(m::Method, kwtype::DataType)
-    sig = rewrap_unionall(Tuple{kwtype, Any, unwrap_unionall(m.sig).parameters...}, m.sig)
-    kwli = ccall(:jl_methtable_lookup, Any, (Any, Any, UInt), kwtype.name.mt, sig, get_world_counter())
-    if kwli !== nothing
-        kwli = kwli::Method
-        slotnames = ccall(:jl_uncompress_argnames, Vector{Any}, (Any,), kwli.slot_syms)
-        kws = filter(x -> !(x === empty_sym || '#' in string(x)), slotnames[(kwli.nargs + 1):end])
-        # ensure the kwarg... is always printed last. The order of the arguments are not
-        # necessarily the same as defined in the function
-        i = findfirst(x -> endswith(string(x), "..."), kws)
-        if i !== nothing
-            push!(kws, kws[i])
-            deleteat!(kws, i)
+function kwarg_decl(m::Method)
+    mt = get_methodtable(m)
+    if isdefined(mt, :kwsorter)
+        kwtype = typeof(mt.kwsorter)
+        sig = rewrap_unionall(Tuple{kwtype, Any, unwrap_unionall(m.sig).parameters...}, m.sig)
+        kwli = ccall(:jl_methtable_lookup, Any, (Any, Any, UInt), kwtype.name.mt, sig, get_world_counter())
+        if kwli !== nothing
+            kwli = kwli::Method
+            slotnames = ccall(:jl_uncompress_argnames, Vector{Any}, (Any,), kwli.slot_syms)
+            kws = filter(x -> !(x === empty_sym || '#' in string(x)), slotnames[(kwli.nargs + 1):end])
+            # ensure the kwarg... is always printed last. The order of the arguments are not
+            # necessarily the same as defined in the function
+            i = findfirst(x -> endswith(string(x), "..."), kws)
+            if i !== nothing
+                push!(kws, kws[i])
+                deleteat!(kws, i)
+            end
+            return kws
         end
-        return kws
     end
-    return ()
+    return Any[]
 end
 
 function show_method_params(io::IO, tv)
@@ -155,7 +159,7 @@ function functionloc(@nospecialize(f))
     return functionloc(first(mt))
 end
 
-function show(io::IO, m::Method; kwtype::Union{DataType, Nothing}=nothing)
+function show(io::IO, m::Method)
     tv, decls, file, line = arg_decl_parts(m)
     sig = unwrap_unionall(m.sig)
     ft0 = sig.parameters[1]
@@ -182,12 +186,10 @@ function show(io::IO, m::Method; kwtype::Union{DataType, Nothing}=nothing)
     print(io, "(")
     join(io, [isempty(d[2]) ? d[1] : d[1]*"::"*d[2] for d in decls[2:end]],
                  ", ", ", ")
-    if kwtype !== nothing
-        kwargs = kwarg_decl(m, kwtype)
-        if !isempty(kwargs)
-            print(io, "; ")
-            join(io, kwargs, ", ", ", ")
-        end
+    kwargs = kwarg_decl(m)
+    if !isempty(kwargs)
+        print(io, "; ")
+        join(io, kwargs, ", ", ", ")
     end
     print(io, ")")
     show_method_params(io, tv)
@@ -235,7 +237,6 @@ function show_method_table(io::IO, ms::MethodList, max::Int=-1, header::Bool=tru
     if header
         show_method_list_header(io, ms, str -> "\""*str*"\"")
     end
-    kwtype = isdefined(mt, :kwsorter) ? typeof(mt.kwsorter) : nothing
     n = rest = 0
     local last
 
@@ -245,7 +246,7 @@ function show_method_table(io::IO, ms::MethodList, max::Int=-1, header::Bool=tru
             n += 1
             println(io)
             print(io, "[$(n)] ")
-            show(io, meth; kwtype=kwtype)
+            show(io, meth)
             file, line = meth.file, meth.line
             try
                 file, line = invokelatest(methodloc_callback[], meth)
@@ -260,7 +261,7 @@ function show_method_table(io::IO, ms::MethodList, max::Int=-1, header::Bool=tru
     if rest > 0
         println(io)
         if rest == 1
-            show(io, last; kwtype=kwtype)
+            show(io, last)
         else
             print(io, "... $rest methods not shown")
             if hasname
@@ -323,7 +324,7 @@ function url(m::Method)
     end
 end
 
-function show(io::IO, ::MIME"text/html", m::Method; kwtype::Union{DataType, Nothing}=nothing)
+function show(io::IO, ::MIME"text/html", m::Method)
     tv, decls, file, line = arg_decl_parts(m)
     sig = unwrap_unionall(m.sig)
     ft0 = sig.parameters[1]
@@ -346,13 +347,11 @@ function show(io::IO, ::MIME"text/html", m::Method; kwtype::Union{DataType, Noth
     print(io, "(")
     join(io, [isempty(d[2]) ? d[1] : d[1]*"::<b>"*d[2]*"</b>"
                       for d in decls[2:end]], ", ", ", ")
-    if kwtype !== nothing
-        kwargs = kwarg_decl(m, kwtype)
-        if !isempty(kwargs)
-            print(io, "; <i>")
-            join(io, kwargs, ", ", ", ")
-            print(io, "</i>")
-        end
+    kwargs = kwarg_decl(m)
+    if !isempty(kwargs)
+        print(io, "; <i>")
+        join(io, kwargs, ", ", ", ")
+        print(io, "</i>")
     end
     print(io, ")")
     if !isempty(tv)
@@ -379,11 +378,10 @@ end
 function show(io::IO, mime::MIME"text/html", ms::MethodList)
     mt = ms.mt
     show_method_list_header(io, ms, str -> "<b>"*str*"</b>")
-    kwtype = isdefined(mt, :kwsorter) ? typeof(mt.kwsorter) : nothing
     print(io, "<ul>")
     for meth in ms
         print(io, "<li> ")
-        show(io, mime, meth; kwtype=kwtype)
+        show(io, mime, meth)
         print(io, "</li> ")
     end
     print(io, "</ul>")

base/reflection.jl

@@ -1220,7 +1220,7 @@ function hasmethod(@nospecialize(f), @nospecialize(t), kwnames::Tuple{Vararg{Sym
     hasmethod(f, t, world=world) || return false
     isempty(kwnames) && return true
     m = which(f, t)
-    kws = kwarg_decl(m, Core.kwftype(typeof(f)))
+    kws = kwarg_decl(m)
     for kw in kws
         endswith(String(kw), "...") && return true
     end

base/show.jl

@@ -1501,11 +1501,21 @@ function show_unquoted(io::IO, ex::Expr, indent::Int, prec::Int)
     nothing
 end
 
-function show_tuple_as_call(io::IO, name::Symbol, sig::Type)
+demangle_function_name(name::Symbol) = Symbol(demangle_function_name(string(name)))
+function demangle_function_name(name::AbstractString)
+    demangle = split(name, '#')
+    # kw sorters and impl methods use the name scheme `f#...`
+    if length(demangle) >= 2 && demangle[1] != ""
+        return demangle[1]
+    end
+    return name
+end
+
+function show_tuple_as_call(io::IO, name::Symbol, sig::Type, demangle=false, kwargs=nothing)
     # print a method signature tuple for a lambda definition
     color = get(io, :color, false) && get(io, :backtrace, false) ? stackframe_function_color() : :nothing
     if sig === Tuple
-        printstyled(io, name, "(...)", color=color)
+        printstyled(io, demangle ? demangle_function_name(name) : name, "(...)", color=color)
         return
     end
     tv = Any[]
@@ -1522,7 +1532,7 @@ function show_tuple_as_call(io::IO, name::Symbol, sig::Type)
         if ft <: Function && isa(uw,DataType) && isempty(uw.parameters) &&
                 isdefined(uw.name.module, uw.name.mt.name) &&
                 ft == typeof(getfield(uw.name.module, uw.name.mt.name))
-            print(io, uw.name.mt.name)
+            print(io, (demangle ? demangle_function_name : identity)(uw.name.mt.name))
         elseif isa(ft, DataType) && ft.name === Type.body.name && !Core.Compiler.has_free_typevars(ft)
             f = ft.parameters[1]
             print(io, f)
@@ -1538,6 +1548,16 @@ function show_tuple_as_call(io::IO, name::Symbol, sig::Type)
         first = false
         print(env_io, "::", sig[i])
     end
+    if kwargs !== nothing
+        print(io, "; ")
+        first = true
+        for (k, t) in kwargs
+            first || print(io, ", ")
+            first = false
+            print(io, k, "::")
+            show(io, t)
+        end
+    end
     printstyled(io, ")", color=print_style)
     show_method_params(io, tv)
     nothing

base/stacktraces.jl

@@ -248,11 +248,28 @@ function show_spec_linfo(io::IO, frame::StackFrame)
             color = get(io, :color, false) && get(io, :backtrace, false) ?
                         Base.stackframe_function_color() :
                         :nothing
-            printstyled(io, string(frame.func), color=color)
+            printstyled(io, Base.demangle_function_name(string(frame.func)), color=color)
         end
     elseif frame.linfo isa Core.MethodInstance
-        if isa(frame.linfo.def, Method)
-            Base.show_tuple_as_call(io, frame.linfo.def.name, frame.linfo.specTypes)
+        def = frame.linfo.def
+        if isa(def, Method)
+            sig = frame.linfo.specTypes
+            if def.nkw > 0
+                # rearrange call kw_impl(kw_args..., func, pos_args...) to func(pos_args...)
+                kwarg_types = Any[ fieldtype(sig, i) for i = 2:(1+def.nkw) ]
+                uw = Base.unwrap_unionall(sig)
+                pos_sig = Base.rewrap_unionall(Tuple{uw.parameters[(def.nkw+2):end]...}, sig)
+                kwnames = Base.method_argnames(def)[2:(def.nkw+1)]
+                for i = 1:length(kwnames)
+                    str = string(kwnames[i])
+                    if endswith(str, "...")
+                        kwnames[i] = Symbol(str[1:end-3])
+                    end
+                end
+                Base.show_tuple_as_call(io, def.name, pos_sig, true, zip(kwnames, kwarg_types))
+            else
+                Base.show_tuple_as_call(io, def.name, sig, true)
+            end
         else
             Base.show(io, frame.linfo)
         end

src/builtins.c

@@ -1293,7 +1293,7 @@ void jl_init_primitives(void) JL_GC_DISABLED
     jl_builtin_applicable = add_builtin_func("applicable", jl_f_applicable);
     jl_builtin_invoke = add_builtin_func("invoke", jl_f_invoke);
     jl_typename_t *itn = ((jl_datatype_t*)jl_typeof(jl_builtin_invoke))->name;
-    jl_value_t *ikws = jl_new_generic_function_with_supertype(itn->name, jl_core_module, jl_builtin_type, 1);
+    jl_value_t *ikws = jl_new_generic_function_with_supertype(itn->name, jl_core_module, jl_builtin_type);
     itn->mt->kwsorter = ikws;
     jl_gc_wb(itn->mt, ikws);
     jl_mk_builtin_func((jl_datatype_t*)jl_typeof(ikws), jl_symbol_name(jl_gf_name(ikws)), jl_f_invoke_kwsorter);

src/datatype.c

@@ -29,7 +29,7 @@ jl_sym_t *jl_demangle_typename(jl_sym_t *s) JL_NOTSAFEPOINT
     if (end == n || end == n+1)
         len = strlen(n) - 1;
     else
-        len = (end-n) - 1;
+        len = (end-n) - 1;  // extract `f` from `#f#...`
     return jl_symbol_n(&n[1], len);
 }
 

src/dump.c

@@ -317,7 +317,9 @@ static void jl_serialize_datatype(jl_serializer_state *s, jl_datatype_t *dt) JL_
         tag = 10;
     }
 
-    if (strncmp(jl_symbol_name(dt->name->name), "#kw#", 4) == 0 && !internal && tag != 0) {
+    char *dtname = jl_symbol_name(dt->name->name);
+    size_t dtnl = strlen(dtname);
+    if (dtnl > 4 && strcmp(&dtname[dtnl - 4], "##kw") == 0 && !internal && tag != 0) {
         /* XXX: yuck, this is horrible, but the auto-generated kw types from the serializer isn't a real type, so we *must* be very careful */
         assert(tag == 6); // other struct types should never exist
         tag = 9;
@@ -335,6 +337,8 @@ static void jl_serialize_datatype(jl_serializer_state *s, jl_datatype_t *dt) JL_
             prefixed = (char*)malloc(l + 2);
             prefixed[0] = '#';
             strcpy(&prefixed[1], jl_symbol_name(mt->name));
+            // remove ##kw suffix
+            prefixed[l-3] = 0;
             jl_sym_t *tname = jl_symbol(prefixed);
             free(prefixed);
             jl_value_t *primarydt = jl_get_global(mt->module, tname);
@@ -821,6 +825,7 @@ static void jl_serialize_value_(jl_serializer_state *s, jl_value_t *v, int as_li
         write_int32(s->s, m->called);
         write_int32(s->s, m->nargs);
         write_int32(s->s, m->nospecialize);
+        write_int32(s->s, m->nkw);
         write_int8(s->s, m->isva);
         write_int8(s->s, m->pure);
         jl_serialize_value(s, (jl_value_t*)m->module);
@@ -1692,6 +1697,7 @@ static jl_value_t *jl_deserialize_value_method(jl_serializer_state *s, jl_value_
     m->called = read_int32(s->s);
     m->nargs = read_int32(s->s);
     m->nospecialize = read_int32(s->s);
+    m->nkw = read_int32(s->s);
     m->isva = read_int8(s->s);
     m->pure = read_int8(s->s);
     m->module = (jl_module_t*)jl_deserialize_value(s, (jl_value_t**)&m->module);

src/gf.c

@@ -137,7 +137,7 @@ jl_datatype_t *jl_mk_builtin_func(jl_datatype_t *dt, const char *name, jl_fptr_a
 {
     jl_sym_t *sname = jl_symbol(name);
     if (dt == NULL) {
-        jl_value_t *f = jl_new_generic_function_with_supertype(sname, jl_core_module, jl_builtin_type, 0);
+        jl_value_t *f = jl_new_generic_function_with_supertype(sname, jl_core_module, jl_builtin_type);
         jl_set_const(jl_core_module, sname, f);
         dt = (jl_datatype_t*)jl_typeof(f);
     }
@@ -2428,21 +2428,14 @@ JL_DLLEXPORT jl_value_t *jl_get_invoke_lambda(jl_typemap_entry_t *entry, jl_valu
 }
 
 // Return value is rooted globally
-jl_function_t *jl_new_generic_function_with_supertype(jl_sym_t *name, jl_module_t *module, jl_datatype_t *st, int iskw)
+jl_function_t *jl_new_generic_function_with_supertype(jl_sym_t *name, jl_module_t *module, jl_datatype_t *st)
 {
     // type name is function name prefixed with #
     size_t l = strlen(jl_symbol_name(name));
     char *prefixed;
-    if (iskw) {
-        prefixed = (char*)malloc(l+5);
-        strcpy(&prefixed[0], "#kw#");
-        strcpy(&prefixed[4], jl_symbol_name(name));
-    }
-    else {
-        prefixed = (char*)malloc(l+2);
-        prefixed[0] = '#';
-        strcpy(&prefixed[1], jl_symbol_name(name));
-    }
+    prefixed = (char*)malloc(l+2);
+    prefixed[0] = '#';
+    strcpy(&prefixed[1], jl_symbol_name(name));
     jl_sym_t *tname = jl_symbol(prefixed);
     free(prefixed);
     jl_datatype_t *ftype = (jl_datatype_t*)jl_new_datatype(
@@ -2466,16 +2459,23 @@ JL_DLLEXPORT jl_function_t *jl_get_kwsorter(jl_value_t *ty)
     if (!mt->kwsorter) {
         JL_LOCK(&mt->writelock);
         if (!mt->kwsorter) {
-            jl_sym_t *name;
+            char *name;
             if (mt == jl_nonfunction_mt) {
-                name = mt->name;
+                name = jl_symbol_name(mt->name);
             }
             else {
                 jl_datatype_t *dt = (jl_datatype_t*)jl_argument_datatype(ty);
                 assert(jl_is_datatype(dt));
-                name = dt->name->name;
+                name = jl_symbol_name(dt->name->name);
+                if (name[0] == '#')
+                    name++;
             }
-            mt->kwsorter = jl_new_generic_function_with_supertype(name, mt->module, jl_function_type, 1);
+            size_t l = strlen(name);
+            char *suffixed = (char*)malloc(l+5);
+            strcpy(&suffixed[0], name);
+            strcpy(&suffixed[l], "##kw");
+            jl_sym_t *fname = jl_symbol(suffixed);
+            mt->kwsorter = jl_new_generic_function_with_supertype(fname, mt->module, jl_function_type);
             jl_gc_wb(mt, mt->kwsorter);
         }
         JL_UNLOCK(&mt->writelock);
@@ -2485,7 +2485,7 @@ JL_DLLEXPORT jl_function_t *jl_get_kwsorter(jl_value_t *ty)
 
 jl_function_t *jl_new_generic_function(jl_sym_t *name, jl_module_t *module)
 {
-    return jl_new_generic_function_with_supertype(name, module, jl_function_type, 0);
+    return jl_new_generic_function_with_supertype(name, module, jl_function_type);
 }
 
 struct ml_matches_env {