From fb9f6c6dc11e6c705759df9f3011fb24d0e096bf Mon Sep 17 00:00:00 2001 From: Peter Ahrens Date: Thu, 30 Dec 2021 17:45:01 -0500 Subject: [PATCH 1/5] Fixes 165, Fixes 177. --- src/utils.jl | 37 ++++++++++++++++++++----------------- test/split.jl | 21 ++++++++++++++++++--- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/src/utils.jl b/src/utils.jl index 357236a..3b5c22a 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -1,5 +1,5 @@ export @esc, isexpr, isline, iscall, rmlines, unblock, block, inexpr, namify, isdef, - longdef, shortdef, @expand, makeif, prettify, combinedef, splitdef, splitarg + longdef, shortdef, @expand, makeif, prettify, combinedef, splitdef, splitarg, combinearg """ assoc!(d, k, v) @@ -406,7 +406,10 @@ end `combinearg` is the inverse of [`splitarg`](@ref). """ function combinearg(arg_name, arg_type, is_splat, default) - a = arg_name===nothing ? :(::$arg_type) : :($arg_name::$arg_type) + @assert arg_name !== nothing || arg_type !== nothing + a = arg_name===nothing ? :(::$arg_type) : + arg_type===nothing ? arg_name : + :($arg_name::$arg_type) a2 = is_splat ? Expr(:..., a) : a return default === nothing ? a2 : Expr(:kw, a2, default) end @@ -415,34 +418,34 @@ end splitarg(arg) Match function arguments (whether from a definition or a function call) such as -`x::Int=2` and return `(arg_name, arg_type, is_splat, default)`. `arg_name` and -`default` are `nothing` when they are absent. For example: +`x::Int=2` and return `(arg_name, arg_type, is_splat, default)`. `arg_name`, +`arg_type`, and `default` are `nothing` when they are absent. For example: ```julia julia> map(splitarg, (:(f(a=2, x::Int=nothing, y, args...))).args[2:end]) 4-element Array{Tuple{Symbol,Symbol,Bool,Any},1}: - (:a, :Any, false, 2) + (:a, nothing, false, 2) (:x, :Int, false, :nothing) (:y, :Any, false, nothing) - (:args, :Any, true, nothing) + (:args, nothing, true, nothing) ``` See also: [`combinearg`](@ref) """ function splitarg(arg_expr) - splitvar(arg) = - (@match arg begin - ::T_ => (nothing, T) - name_::T_ => (name, T) - x_ => (x, :Any) - end)::NTuple{2,Any} # the pattern `x_` matches any expression - (is_splat = @capture(arg_expr, arg_expr2_...)) || (arg_expr2 = arg_expr) - if @capture(arg_expr2, arg_ = default_) - @assert default !== nothing "splitarg cannot handle `nothing` as a default. Use a quoted `nothing` if possible. (MacroTools#35)" - return (splitvar(arg)..., is_splat, default) + if @capture(arg_expr, arg_expr2_ = default_) + @assert default !== nothing "splitarg cannot handle `nothing` as a default. Use a quoted `nothing` if possible. (MacroTools#35)" else - return (splitvar(arg_expr2)..., is_splat, nothing) + arg_expr2 = arg_expr end + is_splat = @capture(arg_expr2, arg_expr3_...) + is_splat || (arg_expr3 = arg_expr2) + (arg_name, arg_type) = (@match arg_expr3 begin + ::T_ => (nothing, T) + name_::T_ => (name, T) + x_ => (x, nothing) + end)::NTuple{2,Any} # the pattern `x_` matches any expression + return (arg_name, arg_type, is_splat, default) end diff --git a/test/split.jl b/test/split.jl index 3e56b79..3c67456 100644 --- a/test/split.jl +++ b/test/split.jl @@ -6,6 +6,8 @@ end macro splitcombine(fundef) # should be a no-op dict = splitdef(fundef) + dict[:args] = map(arg->combinearg(splitarg(arg)...), dict[:args]) + dict[:kwargs] = map(arg->combinearg(splitarg(arg)...), dict[:kwargs]) esc(MacroTools.combinedef(dict)) end @@ -24,10 +26,17 @@ let @test longdef(:(f(x)::Int = 10)).head == :function @test longdef(:(f(x::T) where U where T = 2)).head == :function @test shortdef(:(function f(x)::Int 10 end)).head != :function - @test map(splitarg, (:(f(a=2, x::Int=nothing, y, args...))).args[2:end]) == - [(:a, :Any, false, 2), (:x, :Int, false, :nothing), - (:y, :Any, false, nothing), (:args, :Any, true, nothing)] + @test map(splitarg, (:(f(a=2, x::Int=nothing, y::Any, args...))).args[2:end]) == + [(:a, nothing, false, 2), (:x, :Int, false, :nothing), + (:y, :Any, false, nothing), (:args, nothing, true, nothing)] @test splitarg(:(::Int)) == (nothing, :Int, false, nothing) + kwargs = splitdef(:(f(; a::Int = 1, b...) = 1))[:kwargs] + @test map(splitarg, kwargs) == + [(:a, :Int, false, 1), (:b, nothing, true, nothing)] + args = splitdef(:(f(a::Int = 1) = 1))[:args] + @test map(splitarg, args) == [(:a, :Int, false, 1)] + args = splitdef(:(f(a::Int ... = 1) = 1))[:args] + @test map(splitarg, args) == [(:a, :Int, true, 1)] @splitcombine foo(x) = x+2 @test foo(10) == 12 @@ -47,6 +56,12 @@ let @splitcombine fmacro1() = @onearg 1 @test fmacro1() == 2 + @splitcombine bar(; a::Int = 1, b...) = 2 + @test bar(a=3, x = 1, y = 2) == 2 + @splitcombine qux(a::Int... = 0) = sum(a) + @test qux(1, 2, 3) == 6 + @test qux() == 0 + struct Foo{A, B} a::A b::B From 8da820cdf85f0d9a034f47996e597024ceca57ac Mon Sep 17 00:00:00 2001 From: Peter Ahrens Date: Sat, 1 Jan 2022 10:42:58 -0500 Subject: [PATCH 2/5] No interface change necessary? --- src/utils.jl | 12 ++++++------ test/split.jl | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/utils.jl b/src/utils.jl index 3b5c22a..c4b3acf 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -408,7 +408,7 @@ end function combinearg(arg_name, arg_type, is_splat, default) @assert arg_name !== nothing || arg_type !== nothing a = arg_name===nothing ? :(::$arg_type) : - arg_type===nothing ? arg_name : + arg_type==:Any ? arg_name : :($arg_name::$arg_type) a2 = is_splat ? Expr(:..., a) : a return default === nothing ? a2 : Expr(:kw, a2, default) @@ -418,16 +418,16 @@ end splitarg(arg) Match function arguments (whether from a definition or a function call) such as -`x::Int=2` and return `(arg_name, arg_type, is_splat, default)`. `arg_name`, -`arg_type`, and `default` are `nothing` when they are absent. For example: +`x::Int=2` and return `(arg_name, arg_type, is_splat, default)`. `arg_name` and +`default` are `nothing` when they are absent. For example: ```julia julia> map(splitarg, (:(f(a=2, x::Int=nothing, y, args...))).args[2:end]) 4-element Array{Tuple{Symbol,Symbol,Bool,Any},1}: - (:a, nothing, false, 2) + (:a, :Any, false, 2) (:x, :Int, false, :nothing) (:y, :Any, false, nothing) - (:args, nothing, true, nothing) + (:args, :Any, true, nothing) ``` See also: [`combinearg`](@ref) @@ -443,7 +443,7 @@ function splitarg(arg_expr) (arg_name, arg_type) = (@match arg_expr3 begin ::T_ => (nothing, T) name_::T_ => (name, T) - x_ => (x, nothing) + x_ => (x, :Any) end)::NTuple{2,Any} # the pattern `x_` matches any expression return (arg_name, arg_type, is_splat, default) end diff --git a/test/split.jl b/test/split.jl index 3c67456..482446c 100644 --- a/test/split.jl +++ b/test/split.jl @@ -27,12 +27,12 @@ let @test longdef(:(f(x::T) where U where T = 2)).head == :function @test shortdef(:(function f(x)::Int 10 end)).head != :function @test map(splitarg, (:(f(a=2, x::Int=nothing, y::Any, args...))).args[2:end]) == - [(:a, nothing, false, 2), (:x, :Int, false, :nothing), - (:y, :Any, false, nothing), (:args, nothing, true, nothing)] + [(:a, :Any, false, 2), (:x, :Int, false, :nothing), + (:y, :Any, false, nothing), (:args, :Any, true, nothing)] @test splitarg(:(::Int)) == (nothing, :Int, false, nothing) kwargs = splitdef(:(f(; a::Int = 1, b...) = 1))[:kwargs] @test map(splitarg, kwargs) == - [(:a, :Int, false, 1), (:b, nothing, true, nothing)] + [(:a, :Int, false, 1), (:b, :Any, true, nothing)] args = splitdef(:(f(a::Int = 1) = 1))[:args] @test map(splitarg, args) == [(:a, :Int, false, 1)] args = splitdef(:(f(a::Int ... = 1) = 1))[:args] From f84996f5e08d06799a007ff9dafe79766a0d0084 Mon Sep 17 00:00:00 2001 From: Peter Ahrens Date: Sun, 2 Jan 2022 20:28:15 -0500 Subject: [PATCH 3/5] Add comments suggested by @cstjean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Cédric St-Jean --- src/utils.jl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/utils.jl b/src/utils.jl index c4b3acf..2e4a402 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -434,6 +434,8 @@ See also: [`combinearg`](@ref) """ function splitarg(arg_expr) if @capture(arg_expr, arg_expr2_ = default_) + # This assert will only be triggered if a `nothing` literal was somehow spliced into the Expr. + # A regular `nothing` default value is a `Symbol` when it gets here. See #178 @assert default !== nothing "splitarg cannot handle `nothing` as a default. Use a quoted `nothing` if possible. (MacroTools#35)" else arg_expr2 = arg_expr From 8aefd25264fa6202cd1fba0952229ddef4fa6f6b Mon Sep 17 00:00:00 2001 From: Peter Ahrens Date: Tue, 4 Jan 2022 15:57:50 -0500 Subject: [PATCH 4/5] Only avoid typeassert in combinearg when splatting. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Cédric St-Jean --- src/utils.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.jl b/src/utils.jl index 2e4a402..a562c48 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -408,7 +408,7 @@ end function combinearg(arg_name, arg_type, is_splat, default) @assert arg_name !== nothing || arg_type !== nothing a = arg_name===nothing ? :(::$arg_type) : - arg_type==:Any ? arg_name : + arg_type==:Any && is_splat ? arg_name : # see #177 and julia#43625 :($arg_name::$arg_type) a2 = is_splat ? Expr(:..., a) : a return default === nothing ? a2 : Expr(:kw, a2, default) From 0dea2c093d3f105fa19bbddee5a3d386195d22ec Mon Sep 17 00:00:00 2001 From: Peter Ahrens Date: Tue, 4 Jan 2022 15:59:02 -0500 Subject: [PATCH 5/5] Point out the test for 165 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Cédric St-Jean --- test/split.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/split.jl b/test/split.jl index 482446c..9fc7801 100644 --- a/test/split.jl +++ b/test/split.jl @@ -36,7 +36,7 @@ let args = splitdef(:(f(a::Int = 1) = 1))[:args] @test map(splitarg, args) == [(:a, :Int, false, 1)] args = splitdef(:(f(a::Int ... = 1) = 1))[:args] - @test map(splitarg, args) == [(:a, :Int, true, 1)] + @test map(splitarg, args) == [(:a, :Int, true, 1)] # issue 165 @splitcombine foo(x) = x+2 @test foo(10) == 12