Skip to content

Commit

Permalink
errorshow: simplify printing of keyword argument types using a new ma…
Browse files Browse the repository at this point in the history
…cro format (#49959)

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
  • Loading branch information
aviatesk authored May 31, 2023
1 parent d3d09c1 commit 9b27a8f
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 44 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----------------------------
Expand Down
1 change: 1 addition & 0 deletions base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,7 @@ export
@v_str, # version number
@raw_str, # raw string with no interpolation/unescaping
@NamedTuple,
@Kwargs,
@lazy_str, # lazy string

# documentation
Expand Down
59 changes: 59 additions & 0 deletions base/namedtuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand Down
65 changes: 49 additions & 16 deletions base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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, "; ")
Expand All @@ -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)
Expand Down
1 change: 1 addition & 0 deletions doc/src/base/base.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ Core.Tuple
Core.NTuple
Core.NamedTuple
Base.@NamedTuple
Base.@Kwargs
Base.Val
Core.Vararg
Core.Nothing
Expand Down
86 changes: 58 additions & 28 deletions test/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions test/namedtuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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,)
Expand Down
2 changes: 2 additions & 0 deletions test/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down

2 comments on commit 9b27a8f

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

Please sign in to comment.