From 93557738b9a35856616081b91b6f677d8d26de6f Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 30 Jun 2021 14:05:41 +0100 Subject: [PATCH 1/7] add signature from type tuple --- src/method.jl | 89 +++++++++++++++++++++++++++++++++++++++++++++++++- test/method.jl | 39 ++++++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) diff --git a/src/method.jl b/src/method.jl index 5f4b0c4..6f47502 100644 --- a/src/method.jl +++ b/src/method.jl @@ -37,9 +37,63 @@ function signature(m::Method) def[:params] = type_parameters(m) def[:kwargs] = kwargs(m) - return Dict(k => v for (k, v) in def if v !== nothing) # filter out nonfields. + return filter!(kv->last(kv)!==nothing, def) # filter out nonfields. end + +""" + signature(sig::Type{<:Tuple}) + +Like `ExprTools.signature(::Method)` but on the underlying signature type-tuple, rather than +the Method`. +For `sig` being a tuple-type representing a methods type signature, this generates a +dictionary that can be passes to `ExprTools.combinedef` to define that function, +Provided that you assign the `:body` key on the dictionary first. + +The quality of the output, in terms of matching names etc is not as high as for the +`signature(::Method`; but all the key information is present; and the type-typle is for +other purposes generally easier to manipulate. + +Examples +```julia +julia> signature(Tuple{typeof(identity), Any}) +Dict{Symbol, Any} with 2 entries: + :name => :(op::typeof(identity)) + :args => Expr[:(x1::Any)] + +julia> signature(Tuple{typeof(+), Vector{T}, Vector{T}} where T<:Number) +Dict{Symbol, Any} with 3 entries: + :name => :(op::typeof(+)) + :args => Expr[:(x1::Array{var"##T#5492", 1}), :(x2::Array{var"##T#5492", 1})] + :whereparams => Any[:(var"##T#5492" <: Number)] +``` + +# keywords + + - `hygienic_unionalls=false`: if set to `true` this forces name-hygine on the `TypeVar`s in + `UnionAll`s, regenerating each with a unique name via `gensym`. This shouldn't actually + be requires as they are scoped such that they are not supposed to leak. However, there is + a long standing [julia bug](https://github.com/JuliaLang/julia/issues/39876) that means + they do leak if they clash with function type-vars. +""" +function signature(orig_sig::Type{<:Tuple}; hygienic_unionalls=false) + sig = hygienic_unionalls ? _truly_rename_unionall(orig_sig) : orig_sig + def = Dict{Symbol, Any}() + + opT = parameters(sig)[1] + def[:name] = :(op::$opT) + + arg_types = name_of_type.(argument_types(sig)) + arg_names = [Symbol(:x, ii) for ii in eachindex(arg_types)] + def[:args] = Expr.(:(::), arg_names, arg_types) + def[:whereparams] = where_parameters(sig) + + filter!(kv->last(kv)!==nothing, def) # filter out nonfields. + return def +end + + + function slot_names(m::Method) ci = Base.uncompressed_ast(m) return ci.slotnames @@ -178,3 +232,36 @@ function kwarg_names(m::Method) !isdefined(mt, :kwsorter) && return [] # no kwsorter means no keywords for sure. return Base.kwarg_decl(m, typeof(mt.kwsorter)) end + + + +""" + _truly_rename_unionall(@nospecialize(u)) + +For `u` being a `UnionAll` this replaces every `TypeVar` with a new one with a `gensym`ed +names. +This shouldn't actually be required as they are scoped such that they are not supposed to leak. However, there is +a long standing [julia bug](https://github.com/JuliaLang/julia/issues/39876) that means +they do leak if they clash with function type-vars. + +Example: +```julia +julia> _truly_rename_unionall(Array{T, N} where {T<:Number, N}) +Array{var"##T#2881", var"##N#2880"} where var"##N#2880" where var"##T#2881"<:Number +``` + +Note that the similar `Base.rename_unionall`, though `Base.rename_unionall` does not +`gensym` the names just replaces the instances with new instances with identical names. +""" +function _truly_rename_unionall(@nospecialize(u)) + isa(u,UnionAll) || return u + body = _truly_rename_unionall(u.body) + if body === u.body + body = u + else + body = UnionAll(u.var, body) + end + var = u.var::TypeVar + nv = TypeVar(gensym(var.name), var.lb, var.ub) + return UnionAll(nv, body{nv}) +end diff --git a/test/method.jl b/test/method.jl index c8b2cb0..c316e47 100644 --- a/test/method.jl +++ b/test/method.jl @@ -234,4 +234,43 @@ end only_method(OneParamStruct{Float32}, Tuple{Float32, Bool}) ) end + + @testset "signature(type_tuple)" begin + # our tests here are much less comprehensive than for `signature(::Method)` + # but that is OK, as most of the code is shared between the two + + @test signature(Tuple{typeof(+), Float32, Float32}) == Dict( + :name => :(op::typeof(+)), + :args => Expr[:(x1::Float32), :(x2::Float32)], + ) + + @test signature(Tuple{typeof(+), Array}) == Dict( + :name => :(op::typeof(+)), + :args => Expr[:(x1::(Array{T, N} where {T, N}))], + ) + + @test signature(Tuple{typeof(+), Vector{T}, Matrix{T}} where T<:Real) == Dict( + :name => :(op::typeof(+)), + :args => Expr[:(x1::Array{T, 1}), :(x2::Array{T, 2})], + :whereparams => Any[:(T <: Real)], + ) + + @testset "hygienic_unionalls" begin + no_hygiene = signature(Tuple{typeof(+),T,Array} where T) + @test no_hygiene == Dict( + :name => :(op::typeof(+)), + :args => Expr[:(x1::T), :(x2::(Array{T, N} where {T, N}))], + :whereparams => Any[:T], + ) + hygiene = signature(Tuple{typeof(+),T,Array} where T; hygienic_unionalls=true) + @test no_hygiene[:name] == hygiene[:name] + @test length(no_hygiene[:args]) == 2 + @test no_hygiene[:args][2] == hygiene[:args][2] + + @test length(no_hygiene[:whereparams]) == 1 + @test no_hygiene[:whereparams] != hygiene[:whereparams] + # very coarse test to make sure the renamed arg is in the expression it should be + @test occursin(string(no_hygiene[:whereparams][1]), string(no_hygiene[:args][1])) + end + end end From 47d7e25dc39c2a17ee38de3e59fa682512846a53 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 30 Jun 2021 14:08:26 +0100 Subject: [PATCH 2/7] mention in readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 6b730b0..79aa634 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ Alternatively see the [MacroTools](https://github.com/MikeInnes/MacroTools.jl) p Currently, this package provides the `splitdef`, `signature` and `combinedef` functions which are useful for inspecting and manipulating function definition expressions. - `splitdef` works on a function definition expression and returns a `Dict` of its parts. - `combinedef` takes a `Dict` from `splitdef` and builds it into an expression. - - `signature` works on a `Method` returning a similar `Dict` that holds the parts of the expressions that would form its signature. + - `signature` works on a `Method`, or the type-tuple `sig` field of a method, returning a similar `Dict` that holds the parts of the expressions that would form its signature. As well as several helpers that are useful in combination with them. - `args_tuple_expr` applies to a `Dict` from `splitdef` or `signature` to generate an expression for a tuple of its arguments. From e44af2774c83452b154ed65017178f2c97c73639 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 30 Jun 2021 17:48:22 +0100 Subject: [PATCH 3/7] make tests take into acount that interpolating the function object --- test/method.jl | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/method.jl b/test/method.jl index c316e47..a96d63f 100644 --- a/test/method.jl +++ b/test/method.jl @@ -240,17 +240,20 @@ end # but that is OK, as most of the code is shared between the two @test signature(Tuple{typeof(+), Float32, Float32}) == Dict( - :name => :(op::typeof(+)), + # Notice the type of the function object is actually interpolated in to the Expr + # This is useful because it bypasses julia's pretection for overloading things + # which Nabla (and probably others generating overloads) depends upon + :name => :(op::$(typeof(+))), :args => Expr[:(x1::Float32), :(x2::Float32)], ) @test signature(Tuple{typeof(+), Array}) == Dict( - :name => :(op::typeof(+)), + :name => :(op::$(typeof(+))), :args => Expr[:(x1::(Array{T, N} where {T, N}))], ) @test signature(Tuple{typeof(+), Vector{T}, Matrix{T}} where T<:Real) == Dict( - :name => :(op::typeof(+)), + :name => :(op::$(typeof(+))), :args => Expr[:(x1::Array{T, 1}), :(x2::Array{T, 2})], :whereparams => Any[:(T <: Real)], ) @@ -258,7 +261,7 @@ end @testset "hygienic_unionalls" begin no_hygiene = signature(Tuple{typeof(+),T,Array} where T) @test no_hygiene == Dict( - :name => :(op::typeof(+)), + :name => :(op::$(typeof(+))), :args => Expr[:(x1::T), :(x2::(Array{T, N} where {T, N}))], :whereparams => Any[:T], ) From cc262020e7934c5ca033f89fa77cb850b5d330c5 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Thu, 1 Jul 2021 15:53:16 +0100 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: Miha Zgubic --- src/method.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/method.jl b/src/method.jl index 6f47502..5f39272 100644 --- a/src/method.jl +++ b/src/method.jl @@ -51,7 +51,7 @@ dictionary that can be passes to `ExprTools.combinedef` to define that function, Provided that you assign the `:body` key on the dictionary first. The quality of the output, in terms of matching names etc is not as high as for the -`signature(::Method`; but all the key information is present; and the type-typle is for +`signature(::Method)`, but all the key information is present; and the type-tuple is for other purposes generally easier to manipulate. Examples @@ -72,8 +72,8 @@ Dict{Symbol, Any} with 3 entries: - `hygienic_unionalls=false`: if set to `true` this forces name-hygine on the `TypeVar`s in `UnionAll`s, regenerating each with a unique name via `gensym`. This shouldn't actually - be requires as they are scoped such that they are not supposed to leak. However, there is - a long standing [julia bug](https://github.com/JuliaLang/julia/issues/39876) that means + be required as they are scoped such that they are not supposed to leak. However, there is + a long-standing [julia bug](https://github.com/JuliaLang/julia/issues/39876) that means they do leak if they clash with function type-vars. """ function signature(orig_sig::Type{<:Tuple}; hygienic_unionalls=false) @@ -254,7 +254,7 @@ Note that the similar `Base.rename_unionall`, though `Base.rename_unionall` does `gensym` the names just replaces the instances with new instances with identical names. """ function _truly_rename_unionall(@nospecialize(u)) - isa(u,UnionAll) || return u + isa(u, UnionAll) || return u body = _truly_rename_unionall(u.body) if body === u.body body = u From 453712506927d9761d049498ac478f9e09a261e9 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Thu, 1 Jul 2021 15:56:28 +0100 Subject: [PATCH 5/7] rename to extra-hygine, and add comments --- src/method.jl | 9 ++++++--- test/method.jl | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/method.jl b/src/method.jl index 5f39272..1f13c22 100644 --- a/src/method.jl +++ b/src/method.jl @@ -70,14 +70,14 @@ Dict{Symbol, Any} with 3 entries: # keywords - - `hygienic_unionalls=false`: if set to `true` this forces name-hygine on the `TypeVar`s in + - `extra_hygiene=false`: if set to `true` this forces name-hygine on the `TypeVar`s in `UnionAll`s, regenerating each with a unique name via `gensym`. This shouldn't actually be required as they are scoped such that they are not supposed to leak. However, there is a long-standing [julia bug](https://github.com/JuliaLang/julia/issues/39876) that means they do leak if they clash with function type-vars. """ -function signature(orig_sig::Type{<:Tuple}; hygienic_unionalls=false) - sig = hygienic_unionalls ? _truly_rename_unionall(orig_sig) : orig_sig +function signature(orig_sig::Type{<:Tuple}; extra_hygiene=false) + sig = extra_hygiene ? _truly_rename_unionall(orig_sig) : orig_sig def = Dict{Symbol, Any}() opT = parameters(sig)[1] @@ -254,6 +254,9 @@ Note that the similar `Base.rename_unionall`, though `Base.rename_unionall` does `gensym` the names just replaces the instances with new instances with identical names. """ function _truly_rename_unionall(@nospecialize(u)) + # This works by recursively unwrapping UnionAlls to seperate the TypeVars from body + # changing the name in the TypeVar, and then rewrapping it back up. + # The code is basically the same as `Base.rename_unionall`, but with gensym added isa(u, UnionAll) || return u body = _truly_rename_unionall(u.body) if body === u.body diff --git a/test/method.jl b/test/method.jl index a96d63f..f7bd3ea 100644 --- a/test/method.jl +++ b/test/method.jl @@ -258,14 +258,14 @@ end :whereparams => Any[:(T <: Real)], ) - @testset "hygienic_unionalls" begin + @testset "extra_hygiene" begin no_hygiene = signature(Tuple{typeof(+),T,Array} where T) @test no_hygiene == Dict( :name => :(op::$(typeof(+))), :args => Expr[:(x1::T), :(x2::(Array{T, N} where {T, N}))], :whereparams => Any[:T], ) - hygiene = signature(Tuple{typeof(+),T,Array} where T; hygienic_unionalls=true) + hygiene = signature(Tuple{typeof(+),T,Array} where T; extra_hygiene=true) @test no_hygiene[:name] == hygiene[:name] @test length(no_hygiene[:args]) == 2 @test no_hygiene[:args][2] == hygiene[:args][2] From c8409b40919c76162ba65e2245a5d389957f6ad6 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Thu, 1 Jul 2021 15:58:30 +0100 Subject: [PATCH 6/7] extra tests on changed from hygiene --- test/method.jl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/method.jl b/test/method.jl index f7bd3ea..1df2a36 100644 --- a/test/method.jl +++ b/test/method.jl @@ -268,10 +268,11 @@ end hygiene = signature(Tuple{typeof(+),T,Array} where T; extra_hygiene=true) @test no_hygiene[:name] == hygiene[:name] @test length(no_hygiene[:args]) == 2 - @test no_hygiene[:args][2] == hygiene[:args][2] + @test no_hygiene[:args][1] != hygiene[:args][1] # different Symbols + @test no_hygiene[:arg s][2] == hygiene[:args][2] @test length(no_hygiene[:whereparams]) == 1 - @test no_hygiene[:whereparams] != hygiene[:whereparams] + @test no_hygiene[:whereparams] != hygiene[:whereparams] # different Symbols # very coarse test to make sure the renamed arg is in the expression it should be @test occursin(string(no_hygiene[:whereparams][1]), string(no_hygiene[:args][1])) end From 62f2fe9ded824f1db508557629727a8e81736566 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Thu, 1 Jul 2021 16:51:35 +0100 Subject: [PATCH 7/7] fix typo --- test/method.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/method.jl b/test/method.jl index 1df2a36..fd3bc2d 100644 --- a/test/method.jl +++ b/test/method.jl @@ -269,7 +269,7 @@ end @test no_hygiene[:name] == hygiene[:name] @test length(no_hygiene[:args]) == 2 @test no_hygiene[:args][1] != hygiene[:args][1] # different Symbols - @test no_hygiene[:arg s][2] == hygiene[:args][2] + @test no_hygiene[:args][2] == hygiene[:args][2] @test length(no_hygiene[:whereparams]) == 1 @test no_hygiene[:whereparams] != hygiene[:whereparams] # different Symbols