Implement splitdef/combinedef

[omus]

After looking over the usage of MacroTools in #60 I found that the proposed implementation wasn't handling the various function definitions that can occur. I found the splitdef and combinedef implementations provided by MacroTools but I discovered that those functions were not fully tested and were quite slow. Additionally, the extra dependences MacroTools added seemed unnecessary here.

julia> using BenchmarkTools, MacroTools, Mocking

julia> @btime MacroTools.splitdef(:(f(x) = 1))
  346.797 μs (543 allocations: 35.33 KiB)
Dict{Symbol,Any} with 5 entries:
  :name        => :f
  :args        => Any[:x]
  :kwargs      => Any[]
  :body        => quote…
  :whereparams => ()

julia> @btime Mocking.splitdef(:(f(x) = 1))
  810.213 ns (14 allocations: 1.14 KiB)
Dict{Symbol,Any} with 4 entries:
  :head => :(=)
  :args => Any[:x]
  :body => quote…
  :name => :f

I think it would be good to make these functions into a separate package for general usage but for now they can live in Mocking (I'm not sure what a good name for this package would be).

• Fixes patching with where #61
• Fixes Mocking does not support Nothing kwarg defaults #58

A quick list of issues with the MacroTools variants:

• MacroTools.splitdef does not handle anonymous functions
• MacroTools.splitdef does not handle empty generic functions
• MacroTools.combinedef can only create long-form functions
• MacroTools.combinedef always includes Expr(:parameters, ...)

[iamed2]

Why not a type or a NamedTuple?

EDIT: I see you mutate it

[omus]

I would be good with returning a type. I ended up using a Dict to mostly keep compatibly with MacroTools' version

[codecov]

Codecov Report

Merging #66 into master will increase coverage by 0.61%.
The diff coverage is 96.59%.

@@            Coverage Diff             @@
##           master      #66      +/-   ##
==========================================
+ Coverage   94.44%   95.05%   +0.61%     
==========================================
  Files           5        5              
  Lines         108      182      +74     
==========================================
+ Hits          102      173      +71     
- Misses          6        9       +3

Impacted Files	Coverage Δ
src/Mocking.jl	93.84% <100%> (-0.19%)	⬇️
src/expr.jl	96.59% <96.05%> (-3.41%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5e316e...939a357. Read the comment docs.

[rofinn]

If the objective is to keep the same API as MacroTools then it might be good to make MacroTools a test dependency and also compare all the cases that should be the same and explicitly show which cases MacroTools won't handle.

[omus]

I'm not sure that would be worth the effort. The functions here definitely don't have the same API (note :type is never returned by MacroTools.splitdef). I'll update the description to mention what isn't handled by MacroTools

[omus]

I'll try to put this code into a a separate package. I think ExprTools.jl is a reasonable name. My reason for not putting back into MacroTools.jl is so that we don't have to add unnecessary dependencies here.

I'll note ExprTools.jl already exists and defines splitdef and combinedef but only works on 0.6

[oxinabox]

A few edge cases I think want tests still.

[omus]

I realized I never benchmarked combinedef:

julia> using BenchmarkTools, MacroTools, Mocking

julia> def1 = @btime MacroTools.splitdef(:(f(x) = 1))
  327.960 μs (543 allocations: 35.33 KiB)
Dict{Symbol,Any} with 5 entries:
  :name        => :f
  :args        => Any[:x]
  :kwargs      => Any[]
  :body        => quote…
  :whereparams => ()

julia> def2 = @btime Mocking.splitdef(:(f(x) = 1))
  547.733 ns (12 allocations: 1.09 KiB)
Dict{Symbol,Any} with 4 entries:
  :args => Any[:x]
  :body => quote…
  :name => :f
  :head => :(=)

julia> @btime MacroTools.combinedef($def1)
  675.561 ns (11 allocations: 704 bytes)
:(function f(x; )
      #= REPL[4]:1 =#
      1
  end)

julia> @btime Mocking.combinedef($def2)
  1.524 μs (7 allocations: 416 bytes)
:(f(x) = begin
          #= REPL[7]:1 =#
          1
      end)

I investigated the slow down and it turned out to be a difference between Expr and :(...). After the performance tweak:

julia> @btime MacroTools.combinedef($def1)
  673.331 ns (11 allocations: 704 bytes)
:(function f(x; )
      #= REPL[3]:1 =#
      1
  end)

julia> @btime Mocking.combinedef($def2)
  442.657 ns (7 allocations: 416 bytes)
:(f(x) = begin
          #= REPL[4]:1 =#
          1
      end)

[omus]

Applying fixups before merging.