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.

Sign up for GitHub

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

Jump to bottom

Change keyword arguments back to regular NamedTuples #25711

Closed
wants to merge 2 commits into from
Closed

Change keyword arguments back to regular NamedTuples #25711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d8f6033
Change keyword arguments to regular NamedTuples
ararslan Jan 23, 2018
09e9ec6
Add NEWS entry for kwargs as NamedTuples
ararslan Jan 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions NEWS.md
View file
Open in desktop
Expand Up @@ -196,6 +196,11 @@ Breaking changes

This section lists changes that do not have deprecation warnings.

* Keyword arguments are now `NamedTuple`s. This is a breaking change because iterating
`NamedTuple`s produces values only, whereas previously iteration over keyword arguments
inside of functions produced name, value pairs. The `pairs` function can be used to
extract both names and values. ([#24795])

* `readuntil` now does *not* include the delimiter in its result, matching the
behavior of `readline`. Pass `keep=true` to get the old behavior ([#25633]).

Expand Down Expand Up @@ -1221,6 +1226,7 @@ Command-line option changes
[#24785]: https://github.com/JuliaLang/julia/issues/24785
[#24786]: https://github.com/JuliaLang/julia/issues/24786
[#24794]: https://github.com/JuliaLang/julia/issues/24794
[#24795]: https://github.com/JuliaLang/julia/issues/24795
[#24805]: https://github.com/JuliaLang/julia/issues/24805
[#24808]: https://github.com/JuliaLang/julia/issues/24808
[#24831]: https://github.com/JuliaLang/julia/issues/24831
Expand Down
10 changes: 5 additions & 5 deletions base/errorshow.jl
View file
Open in desktop
Expand Up @@ -164,13 +164,13 @@ function showerror(io::IO, ex::MethodError)
ft = typeof(f)
name = ft.name.mt.name
f_is_function = false
kwargs = ()
kwargs = NamedTuple()
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Also need to change the line below that was changed by #25658.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Does it look right now?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

This file in the revert is wrong. We want kwargs to be a non-specialized container so that we don't compile a new copy of show_method_candidates.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Wasn't it already that way though? IndexValue is also specialized.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I'm not saying I necessarily managed to catch all of this regression before. In v0.6, we copied it to a Vector{Any}. That's not strictly necessary though, as long we don't pass this as an argument to something that'll cause code-generation-specialization of our error-printing code.

if startswith(string(ft.name.name), "#kw#")
f = ex.args[2]
ft = typeof(f)
name = ft.name.mt.name
arg_types_param = arg_types_param[3:end]
kwargs = pairs(ex.args[1])
kwargs = ex.args[1]
ex = MethodError(f, ex.args[3:end])
end
if f == Base.convert && length(arg_types_param) == 2 && !is_arg_types
Expand Down Expand Up @@ -214,7 +214,7 @@ function showerror(io::IO, ex::MethodError)
end
if !isempty(kwargs)
print(io, "; ")
for (i, (k, v)) in enumerate(kwargs)
for (i, (k, v)) in enumerate(pairs(kwargs))
print(io, k, "=")
show(IOContext(io, :limit => true), v)
i == length(kwargs) || print(io, ", ")
Expand Down Expand Up @@ -294,7 +294,7 @@ function showerror_nostdio(err, msg::AbstractString)
ccall(:jl_printf, Cint, (Ptr{Cvoid},Cstring), stderr_stream, "\n")
end

function show_method_candidates(io::IO, ex::MethodError, @nospecialize kwargs=())
function show_method_candidates(io::IO, ex::MethodError, kwargs::NamedTuple = NamedTuple())
is_arg_types = isa(ex.args, DataType)
arg_types = is_arg_types ? ex.args : typesof(ex.args...)
arg_types_param = Any[arg_types.parameters...]
Expand Down Expand Up @@ -425,7 +425,7 @@ function show_method_candidates(io::IO, ex::MethodError, @nospecialize kwargs=()
if !isempty(kwargs)
unexpected = Symbol[]
if isempty(kwords) || !(any(endswith(string(kword), "...") for kword in kwords))
for (k, v) in kwargs
for (k, v) in pairs(kwargs)
if !(k in kwords)
push!(unexpected, k)
end
Expand Down
2 changes: 1 addition & 1 deletion base/interactiveutil.jl
View file
Open in desktop
Expand Up @@ -420,7 +420,7 @@ function gen_call_with_extracted_types(__module__, fcn, ex0)
return quote
local arg1 = $(esc(args[1]))
$(fcn)(Core.kwfunc(arg1),
Tuple{Any, Core.Typeof(arg1),
Tuple{NamedTuple, Core.Typeof(arg1),
$(typesof)($(map(esc, args[2:end])...)).parameters...})
end
elseif ex0.head == :call
Expand Down
2 changes: 1 addition & 1 deletion base/methodshow.jl
View file
Open in desktop
Expand Up @@ -75,7 +75,7 @@ function arg_decl_parts(m::Method)
end

function kwarg_decl(m::Method, kwtype::DataType)
sig = rewrap_unionall(Tuple{kwtype, Any, unwrap_unionall(m.sig).parameters...}, m.sig)
sig = rewrap_unionall(Tuple{kwtype, NamedTuple, unwrap_unionall(m.sig).parameters...}, m.sig)
kwli = ccall(:jl_methtable_lookup, Any, (Any, Any, UInt), kwtype.name.mt, sig, typemax(UInt))
if kwli !== nothing
kwli = kwli::Method
Expand Down
15 changes: 7 additions & 8 deletions src/julia-syntax.scm
View file
Open in desktop
Expand Up @@ -478,7 +478,7 @@
,(let (;; call mangled(vals..., [rest_kw,] pargs..., [vararg]...)
(ret `(return (call ,mangled
,@(if ordered-defaults keynames vals)
,@(if (null? restkw) '() `((call (top pairs) (call (core NamedTuple)))))
,@(if (null? restkw) '() `((call (core NamedTuple))))
,@(map arg-name pargl)
,@(if (null? vararg) '()
(list `(... ,(arg-name (car vararg)))))))))
Expand Down Expand Up @@ -513,13 +513,13 @@
`((|::|
;; if there are optional positional args, we need to be able to reference the function name
,(if (any kwarg? pargl) (gensy) UNUSED)
(call (core kwftype) ,ftype)) ,kw ,@pargl ,@vararg)
(call (core kwftype) ,ftype)) (:: ,kw (core NamedTuple)) ,@pargl ,@vararg)
`(block
,(scopenest
keynames
(map (lambda (v dflt)
(let* ((k (decl-var v))
(rval0 `(call (top getindex) ,kw (inert ,k)))
(rval0 `(call (top getfield) ,kw (quote ,k)))
;; note: if the "declared" type of a KW arg includes something
;; from keyword-sparams then don't assert it here, since those
;; static parameters don't have values yet. instead, the type
Expand Down Expand Up @@ -548,11 +548,10 @@
,dflt)))
vars vals)
`(block
(= ,rkw (call (top pairs)
,(if (null? keynames)
kw
`(call (top structdiff) ,kw (curly (core NamedTuple)
(tuple ,@(map quotify keynames)))))))
(= ,rkw ,(if (null? keynames)
kw
`(call (top structdiff) ,kw (curly (core NamedTuple)
(tuple ,@(map quotify keynames))))))
,@(if (null? restkw)
`((if (call (top isempty) ,rkw)
(null)
Expand Down
4 changes: 2 additions & 2 deletions stdlib/DelimitedFiles/src/DelimitedFiles.jl
View file
Open in desktop
Expand Up @@ -483,8 +483,8 @@ const valid_opts = [:header, :has_header, :use_mmap, :quotes, :comments, :dims,
const valid_opt_types = [Bool, Bool, Bool, Bool, Bool, NTuple{2,Integer}, Char, Integer, Bool]

function val_opts(opts)
d = Dict{Symbol, Union{Bool, NTuple{2, Integer}, Char, Integer}}()
for (opt_name, opt_val) in opts
d = Dict{Symbol,Union{Bool,NTuple{2,Integer},Char,Integer}}()
for (opt_name, opt_val) in pairs(opts)
in(opt_name, valid_opts) ||
throw(ArgumentError("unknown option $opt_name"))
opt_typ = valid_opt_types[findfirst(equalto(opt_name), valid_opts)::Int]
Expand Down
2 changes: 1 addition & 1 deletion stdlib/Distributed/src/cluster.jl
View file
Open in desktop
Expand Up @@ -370,7 +370,7 @@ function addprocs(manager::ClusterManager; kwargs...)
end

function addprocs_locked(manager::ClusterManager; kwargs...)
params = merge(default_addprocs_params(), AnyDict(kwargs))
params = merge(default_addprocs_params(), AnyDict(pairs(kwargs)))
topology(Symbol(params[:topology]))

if PGRP.topology != :all_to_all
Expand Down
2 changes: 1 addition & 1 deletion test/deprecation_exec.jl
View file
Open in desktop
Expand Up @@ -103,7 +103,7 @@ f25130()
testlogs = testlogger.logs
@test length(testlogs) == 2
@test testlogs[1].id != testlogs[2].id
@test testlogs[1].kwargs[:caller].func == Symbol("top-level scope")
@test testlogs[1].kwargs.caller.func == Symbol("top-level scope")
@test all(l.message == "f25130 message" for l in testlogs)
global_logger(prev_logger)

Expand Down
8 changes: 4 additions & 4 deletions test/errorshow.jl
View file
Open in desktop
Expand Up @@ -116,11 +116,11 @@ error_out3 = String(take!(buf))

c7line = @__LINE__() + 1
method_c7(a, b; kargs...) = a
Base.show_method_candidates(buf, MethodError(method_c7, (1, 1)), pairs((x = 1, y = 2)))
Base.show_method_candidates(buf, MethodError(method_c7, (1, 1)), (x = 1, y = 2))
@test String(take!(buf)) == "\nClosest candidates are:\n method_c7(::Any, ::Any; kargs...)$cfile$c7line"
c8line = @__LINE__() + 1
method_c8(a, b; y=1, w=1) = a
Base.show_method_candidates(buf, MethodError(method_c8, (1, 1)), pairs((x = 1, y = 2, z = 1, w = 1)))
Base.show_method_candidates(buf, MethodError(method_c8, (1, 1)), (x = 1, y = 2, z = 1, w = 1))
@test String(take!(buf)) == "\nClosest candidates are:\n method_c8(::Any, ::Any; y, w)$cfile$c8line got unsupported keyword arguments \"x\", \"z\""

let no_kwsorter_match, e
Expand All @@ -134,7 +134,7 @@ ac15639line = @__LINE__
addConstraint_15639(c::Int32) = c
addConstraint_15639(c::Int64; uncset=nothing) = addConstraint_15639(Int32(c), uncset=uncset)

Base.show_method_candidates(buf, MethodError(addConstraint_15639, (Int32(1),)), pairs((uncset = nothing,)))
Base.show_method_candidates(buf, MethodError(addConstraint_15639, (Int32(1),)), (uncset = nothing,))
@test String(take!(buf)) == "\nClosest candidates are:\n addConstraint_15639(::Int32)$cfile$(ac15639line + 1) got unsupported keyword argument \"uncset\"\n addConstraint_15639(!Matched::Int64; uncset)$cfile$(ac15639line + 2)"

macro except_str(expr, err_type)
Expand Down Expand Up @@ -487,7 +487,7 @@ foo_9965(x::Int) = 2x
end
@test typeof(ex) == MethodError
io = IOBuffer()
Base.show_method_candidates(io, ex, pairs((w = true,)))
Base.show_method_candidates(io, ex, (w = true,))
@test contains(String(take!(io)), "got unsupported keyword argument \"w\"")
end

Expand Down
6 changes: 3 additions & 3 deletions test/keywordargs.jl
View file
Open in desktop
Expand Up @@ -188,7 +188,7 @@ function test4974(;kwargs...)
end
end

@test test4974(a=1) == (2, pairs((a=1,)))
@test test4974(a=1) == (2, (a=1,))

@testset "issue #7704, computed keywords" begin
@test kwf1(1; :tens => 2) == 21
Expand Down Expand Up @@ -300,11 +300,11 @@ end
counter += 1
return counter
end
f(args...; kws...) = (args, values(kws))
f(args...; kws...) = (args, kws)
@test f(get_next(), a=get_next(), get_next(),
b=get_next(), get_next(),
[get_next(), get_next()]...; c=get_next(),
(d = get_next(), f = get_next())...) ==
((1, 3, 5, 6, 7),
((1,3,5,6,7),
(a = 2, b = 4, c = 8, d = 9, f = 10))
end
2 changes: 1 addition & 1 deletion test/syntax.jl
View file
Open in desktop
Expand Up @@ -839,7 +839,7 @@ let f = function (x; kw...)
g = function (x; a = 2)
return (x, a)
end
@test f(1) == (1, pairs(NamedTuple()))
@test f(1) == (1, NamedTuple())
@test g(1) == (1, 2)
end

Expand Down