From 9b27a8f8a5b8a96782379b8ce32fe670d131012b Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> Date: Thu, 1 Jun 2023 00:49:36 +0900 Subject: [PATCH] errorshow: simplify printing of keyword argument types using a new macro format (#49959) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Julia, keyword arguments are represented as `Base.Pairs` objects. However, the object type often appears unnecessarily complex, especially when printed in a stack trace. This commit aims to simplify the printing of stack traces that involve keyword method calls, while still allowing us to reconstruct the actual method signature types from the printed signature types. The approach is similar to #49117: this commit introduces a new macro called `Base.@Kwargs`. It follows the same syntax as `@NamedTuple` and returns a `Base.Pairs` type that is used for keyword method calls. We use this syntax when printing keyword argument types. Here's an example of a stack trace: ```diff diff --git a/b.jl b/a.jl index 91dd6f0464..b804ae4be5 100644 --- a/b.jl +++ b/a.jl @@ -22,12 +22,11 @@ Stacktrace: @ Base ./reduce.jl:44 [inlined] [6] mapfoldl(f::typeof(identity), op::typeof(Base.add_sum), itr::String; init::Int64) @ Base ./reduce.jl:175 [inlined] - [7] mapreduce(f::typeof(identity), op::typeof(Base.add_sum), itr::String; kw::Base.Pairs{…}) + [7] mapreduce(f::typeof(identity), op::typeof(Base.add_sum), itr::String; kw::@Kwargs{init::Int64}) @ Base ./reduce.jl:307 [inlined] - [8] sum(f::typeof(identity), a::String; kw::Base.Pairs{Symbol, Int64, Tuple{Symbol}, @NamedTuple{init::Int64}}) + [8] sum(f::typeof(identity), a::String; kw::@Kwargs{init::Int64}) @ Base ./reduce.jl:535 [inlined] - [9] sum(a::String; kw::Base.Pairs{Symbol, Int64, Tuple{Symbol}, @NamedTuple{init::Int64}}) + [9] sum(a::String; kw::@Kwargs{init::Int64}) @ Base ./reduce.jl:564 [inlined] [10] top-level scope ``` --- * RFC: errorshow: simplify printing of keyword argument types using a new macro format * export and document `Base.@Kwargs` and further simplify the stack trace view * use the `@Kwargs` syntax only when printing kwmethod signature within stack trace view And add tests. * add news entry * more type stability * Apply suggestions from code review * enable the type-repr simplification unconditionally in the stack trace Since keyword pairs can appear within positional arguments, it can be confusing if we print the same type with different representations. * omit type annotation for splat keyword argument * add test for `@Kwargs` * clean up test/errorshow.jl --- NEWS.md | 3 ++ base/exports.jl | 1 + base/namedtuple.jl | 59 ++++++++++++++++++++++++++++++ base/show.jl | 65 ++++++++++++++++++++++++--------- doc/src/base/base.md | 1 + test/errorshow.jl | 86 +++++++++++++++++++++++++++++--------------- test/namedtuple.jl | 6 ++++ test/show.jl | 2 ++ 8 files changed, 179 insertions(+), 44 deletions(-) diff --git a/NEWS.md b/NEWS.md index 404b2b11687af..2e1fa8c102461 100644 --- a/NEWS.md +++ b/NEWS.md @@ -18,6 +18,9 @@ Language changes that significantly improves load and inference times for heavily overloaded methods that dispatch on Types (such as traits and constructors). * The "h bar" `ℏ` (`\hslash` U+210F) character is now treated as equivalent to `ħ` (`\hbar` U+0127). +* When a method with keyword arguments is displayed in the stack trace view, the textual + representation of the keyword arguments' types is simplified using the new + `@Kwargs{key1::Type1, ...}` macro syntax ([#49959]). Compiler/Runtime improvements ----------------------------- diff --git a/base/exports.jl b/base/exports.jl index ec151df0bfde2..8d8983950fe74 100644 --- a/base/exports.jl +++ b/base/exports.jl @@ -1004,6 +1004,7 @@ export @v_str, # version number @raw_str, # raw string with no interpolation/unescaping @NamedTuple, + @Kwargs, @lazy_str, # lazy string # documentation diff --git a/base/namedtuple.jl b/base/namedtuple.jl index 320d068205a3d..5f6bdefbefd75 100644 --- a/base/namedtuple.jl +++ b/base/namedtuple.jl @@ -495,6 +495,65 @@ macro NamedTuple(ex) return :(NamedTuple{($(vars...),), Tuple{$(types...)}}) end +""" + @Kwargs{key1::Type1, key2::Type2, ...} + +This macro gives a convenient way to construct the type representation of keyword arguments +from the same syntax as [`@NamedTuple`](@ref). +For example, when we have a function call like `func([positional arguments]; kw1=1.0, kw2="2")`, +we can use this macro to construct the internal type representation of the keyword arguments +as `@Kwargs{kw1::Float64, kw2::String}`. +The macro syntax is specifically designed to simplify the signature type of a keyword method +when it is printed in the stack trace view. + +```julia +julia> @Kwargs{init::Int} # the internal representation of keyword arguments +Base.Pairs{Symbol, Int64, Tuple{Symbol}, @NamedTuple{init::Int64}} + +julia> sum("julia"; init=1) +ERROR: MethodError: no method matching +(::Char, ::Char) + +Closest candidates are: + +(::Any, ::Any, ::Any, ::Any...) + @ Base operators.jl:585 + +(::Integer, ::AbstractChar) + @ Base char.jl:247 + +(::T, ::Integer) where T<:AbstractChar + @ Base char.jl:237 + +Stacktrace: + [1] add_sum(x::Char, y::Char) + @ Base ./reduce.jl:24 + [2] BottomRF + @ Base ./reduce.jl:86 [inlined] + [3] _foldl_impl(op::Base.BottomRF{typeof(Base.add_sum)}, init::Int64, itr::String) + @ Base ./reduce.jl:62 + [4] foldl_impl(op::Base.BottomRF{typeof(Base.add_sum)}, nt::Int64, itr::String) + @ Base ./reduce.jl:48 [inlined] + [5] mapfoldl_impl(f::typeof(identity), op::typeof(Base.add_sum), nt::Int64, itr::String) + @ Base ./reduce.jl:44 [inlined] + [6] mapfoldl(f::typeof(identity), op::typeof(Base.add_sum), itr::String; init::Int64) + @ Base ./reduce.jl:175 [inlined] + [7] mapreduce(f::typeof(identity), op::typeof(Base.add_sum), itr::String; kw::@Kwargs{init::Int64}) + @ Base ./reduce.jl:307 [inlined] + [8] sum(f::typeof(identity), a::String; kw::@Kwargs{init::Int64}) + @ Base ./reduce.jl:535 [inlined] + [9] sum(a::String; kw::@Kwargs{init::Int64}) + @ Base ./reduce.jl:564 [inlined] + [10] top-level scope + @ REPL[12]:1 +``` + +!!! compat "Julia 1.10" + This macro is available as of Julia 1.10. +""" +macro Kwargs(ex) + return :(let + NT = @NamedTuple $ex + Base.Pairs{keytype(NT),eltype(NT),typeof(NT.parameters[1]),NT} + end) +end + @constprop :aggressive function split_rest(t::NamedTuple{names}, n::Int, st...) where {names} _check_length_split_rest(length(t), n) names_front, names_last_n = split_rest(names, n, st...) diff --git a/base/show.jl b/base/show.jl index 2930b4951dd0a..45d6a502619db 100644 --- a/base/show.jl +++ b/base/show.jl @@ -1057,10 +1057,27 @@ function show_type_name(io::IO, tn::Core.TypeName) nothing end +function maybe_kws_nt(x::DataType) + x.name === typename(Pairs) || return nothing + length(x.parameters) == 4 || return nothing + x.parameters[1] === Symbol || return nothing + p4 = x.parameters[4] + if (isa(p4, DataType) && p4.name === typename(NamedTuple) && length(p4.parameters) == 2) + syms, types = p4.parameters + types isa DataType || return nothing + x.parameters[2] === eltype(p4) || return nothing + isa(syms, Tuple) || return nothing + x.parameters[3] === typeof(syms) || return nothing + return p4 + end + return nothing +end + function show_datatype(io::IO, x::DataType, wheres::Vector{TypeVar}=TypeVar[]) parameters = x.parameters::SimpleVector istuple = x.name === Tuple.name isnamedtuple = x.name === typename(NamedTuple) + kwsnt = maybe_kws_nt(x) n = length(parameters) # Print tuple types with homogeneous tails longer than max_n compactly using `NTuple` or `Vararg` @@ -1094,30 +1111,41 @@ function show_datatype(io::IO, x::DataType, wheres::Vector{TypeVar}=TypeVar[]) return elseif isnamedtuple syms, types = parameters - first = true if syms isa Tuple && types isa DataType print(io, "@NamedTuple{") - for i in 1:length(syms) - if !first - print(io, ", ") - end - print(io, syms[i]) - typ = types.parameters[i] - if typ !== Any - print(io, "::") - show(io, typ) - end - first = false - end + show_at_namedtuple(io, syms, types) print(io, "}") return end + elseif get(io, :backtrace, false)::Bool && kwsnt !== nothing + # simplify the type representation of keyword arguments + # when printing signature of keyword method in the stack trace + print(io, "@Kwargs{") + show_at_namedtuple(io, kwsnt.parameters[1]::Tuple, kwsnt.parameters[2]::DataType) + print(io, "}") + return end show_type_name(io, x.name) show_typeparams(io, parameters, (unwrap_unionall(x.name.wrapper)::DataType).parameters, wheres) end +function show_at_namedtuple(io::IO, syms::Tuple, types::DataType) + first = true + for i in 1:length(syms) + if !first + print(io, ", ") + end + print(io, syms[i]) + typ = types.parameters[i] + if typ !== Any + print(io, "::") + show(io, typ) + end + first = false + end +end + function show_supertypes(io::IO, typ::DataType) print(io, typ) while typ != Any @@ -2508,7 +2536,7 @@ function show_tuple_as_call(out::IO, name::Symbol, sig::Type; print_within_stacktrace(io, argnames[i]; color=:light_black) end print(io, "::") - print_type_bicolor(env_io, sig[i]; use_color = get(io, :backtrace, false)) + print_type_bicolor(env_io, sig[i]; use_color = get(io, :backtrace, false)::Bool) end if kwargs !== nothing print(io, "; ") @@ -2517,8 +2545,13 @@ function show_tuple_as_call(out::IO, name::Symbol, sig::Type; first || print(io, ", ") first = false print_within_stacktrace(io, k; color=:light_black) - print(io, "::") - print_type_bicolor(io, t; use_color = get(io, :backtrace, false)) + if t == pairs(NamedTuple) + # omit type annotation for splat keyword argument + print(io, "...") + else + print(io, "::") + print_type_bicolor(io, t; use_color = get(io, :backtrace, false)::Bool) + end end end print_within_stacktrace(io, ")", bold=true) diff --git a/doc/src/base/base.md b/doc/src/base/base.md index 5556578bcc245..3d17665190e21 100644 --- a/doc/src/base/base.md +++ b/doc/src/base/base.md @@ -234,6 +234,7 @@ Core.Tuple Core.NTuple Core.NamedTuple Base.@NamedTuple +Base.@Kwargs Base.Val Core.Vararg Core.Nothing diff --git a/test/errorshow.jl b/test/errorshow.jl index 5c6d8e3bea08c..9be3e675cede3 100644 --- a/test/errorshow.jl +++ b/test/errorshow.jl @@ -957,43 +957,73 @@ end f_internal_wrap(g, a; kw...) = error(); @inline f_internal_wrap(a; kw...) = f_internal_wrap(identity, a; kw...); -bt = try - f_internal_wrap(1) -catch - catch_backtrace() +let bt + @test try + f_internal_wrap(1) + false + catch + bt = catch_backtrace() + true + end + @test !occursin("#f_internal_wrap#", sprint(Base.show_backtrace, bt)) end -@test !occursin("#f_internal_wrap#", sprint(Base.show_backtrace, bt)) g_collapse_pos(x, y=1.0, z=2.0) = error() -bt = try - g_collapse_pos(1.0) -catch - catch_backtrace() +let bt + @test try + g_collapse_pos(1.0) + false + catch + bt = catch_backtrace() + true + end + bt_str = sprint(Base.show_backtrace, bt) + @test occursin("g_collapse_pos(x::Float64, y::Float64, z::Float64)", bt_str) + @test !occursin("g_collapse_pos(x::Float64)", bt_str) end -bt_str = sprint(Base.show_backtrace, bt) -@test occursin("g_collapse_pos(x::Float64, y::Float64, z::Float64)", bt_str) -@test !occursin("g_collapse_pos(x::Float64)", bt_str) g_collapse_kw(x; y=2.0) = error() -bt = try - g_collapse_kw(1.0) -catch - catch_backtrace() +let bt + @test try + g_collapse_kw(1.0) + false + catch + bt = catch_backtrace() + true + end + bt_str = sprint(Base.show_backtrace, bt) + @test occursin("g_collapse_kw(x::Float64; y::Float64)", bt_str) + @test !occursin("g_collapse_kw(x::Float64)", bt_str) end -bt_str = sprint(Base.show_backtrace, bt) -@test occursin("g_collapse_kw(x::Float64; y::Float64)", bt_str) -@test !occursin("g_collapse_kw(x::Float64)", bt_str) g_collapse_pos_kw(x, y=1.0; z=2.0) = error() -bt = try - g_collapse_pos_kw(1.0) -catch - catch_backtrace() -end -bt_str = sprint(Base.show_backtrace, bt) -@test occursin("g_collapse_pos_kw(x::Float64, y::Float64; z::Float64)", bt_str) -@test !occursin("g_collapse_pos_kw(x::Float64, y::Float64)", bt_str) -@test !occursin("g_collapse_pos_kw(x::Float64)", bt_str) +let bt + @test try + g_collapse_pos_kw(1.0) + false + catch + bt = catch_backtrace() + true + end + bt_str = sprint(Base.show_backtrace, bt) + @test occursin("g_collapse_pos_kw(x::Float64, y::Float64; z::Float64)", bt_str) + @test !occursin("g_collapse_pos_kw(x::Float64, y::Float64)", bt_str) + @test !occursin("g_collapse_pos_kw(x::Float64)", bt_str) +end + +simplify_kwargs_type(pos; kws...) = (pos, sum(kws)) +let bt + res = try + simplify_kwargs_type(0; kw1=1.0, kw2="2.0") + false + catch + bt = catch_backtrace() + true + end + @test res + bt_str = sprint(Base.show_backtrace, bt) + @test occursin("simplify_kwargs_type(pos::$Int; kws::@Kwargs{kw1::Float64, kw2::String})", bt_str) +end # Test Base.print_with_compare in convert MethodErrors struct TypeCompareError{A,B} <: Exception end diff --git a/test/namedtuple.jl b/test/namedtuple.jl index ea3a5cdbb8ee4..eb3846c8cbffd 100644 --- a/test/namedtuple.jl +++ b/test/namedtuple.jl @@ -342,6 +342,12 @@ end @test_throws LoadError include_string(Main, "@NamedTuple(a::Int, b)") end +# @Kwargs +@testset "@Kwargs" begin + @test @Kwargs{a::Int,b::String} == typeof(pairs((;a=1,b="2"))) + @test @Kwargs{} == typeof(pairs((;))) +end + # issue #29333, implicit names let x = 1, y = 2 @test (;y) === (y = 2,) diff --git a/test/show.jl b/test/show.jl index 6949db4bb9956..f2c553b3ff49a 100644 --- a/test/show.jl +++ b/test/show.jl @@ -1369,6 +1369,8 @@ test_repr("(:).a") @test repr(@NamedTuple{kw::NTuple{7, Int64}}) == "@NamedTuple{kw::NTuple{7, Int64}}" @test repr(@NamedTuple{a::Float64, b}) == "@NamedTuple{a::Float64, b}" +# Test general printing of `Base.Pairs` (it should not use the `@Kwargs` macro syntax) +@test repr(@Kwargs{init::Int}) == "Base.Pairs{Symbol, $Int, Tuple{Symbol}, @NamedTuple{init::$Int}}" @testset "issue #42931" begin @test repr(NTuple{4, :A}) == "NTuple{4, :A}"