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.

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

Nested macros using splitdef/combinedef #10

Open
tk3369 opened this issue Jul 7, 2020 · 7 comments
Open

Nested macros using splitdef/combinedef #10

tk3369 opened this issue Jul 7, 2020 · 7 comments

Comments

@tk3369
Copy link

tk3369 commented Jul 7, 2020

I happened to build two macros that uses splitdef and combinedef functions. When I tried to use them together, splitdef gives an error. Looks like it doesn't expand the inner macro before applying the outer macro. Is this a ExprTools problem or could I fix it some other way? thanks

Here's a MWE:

macro foo(ex)
    esc(combinedef(splitdef(ex)))
end

macro bar(ex)
    esc(combinedef(splitdef(ex)))
end

Error as follows:

julia> @foo @bar f(x)
ERROR: LoadError: ArgumentError: Function definition contains invalid function head `:macrocall`
Expr
  head: Symbol macrocall
  args: Array{Any}((3,))
    1: Symbol @bar
    2: LineNumberNode
      line: Int64 1
      file: Symbol REPL[52]
    3: Expr
      head: Symbol call
      args: Array{Any}((2,))
        1: Symbol f
        2: Symbol x

Stacktrace:
 [1] invalid_def at /Users/tomkwong/.julia/packages/ExprTools/xk0OO/src/function.jl:30 [inlined]
 [2] splitdef(::Expr; throw::Bool) at /Users/tomkwong/.julia/packages/ExprTools/xk0OO/src/function.jl:37
 [3] splitdef(::Expr) at /Users/tomkwong/.julia/packages/ExprTools/xk0OO/src/function.jl:24
 [4] @foo(::LineNumberNode, ::Module, ::Any) at ./REPL[40]:2
in expression starting at REPL[52]:1
@omus
Copy link
Contributor

omus commented Jul 8, 2020

The splitdef function is throwing an error here as it only can handle processing of function definitions. What I would do in this case is have your macro remove any macros applied to this function before passing the expression to splitdef. A naive demonstration:

julia> macro foo(ex)
           def = ex
           while def.head === :macrocall
               def = def.args[3]
           end
           esc(combinedef(splitdef(def)))
       end
@foo (macro with 1 method)

julia> @foo @foo f(x) = 1
f (generic function with 1 method)

The implementation above is quite frail and doesn't re-insert themacrocalls found

@tk3369
Copy link
Author

tk3369 commented Jul 10, 2020

That's a good idea and I can make it work that way. It feels still a little awkward as I'm creating two separate packages and they have their own macros using splitdef/combinedef. Let's just call these macros @foo and @bar. The issue is that I don't know if a user will do @foo @bar f(x) = 1 or @bar @foo f(x) = 1. So, I would have to handle this case in both macros.

What do you think about making splitdef auto-expand if a macrocall expression is passed?

@tk3369
Copy link
Author

tk3369 commented Jul 10, 2020

Like this... it would be non-breaking but a mod (module) argument must be passed in order to expand the macro.

function splitdef(ex::Expr; throw::Bool=true, mod=nothing)

    # Auto-expand macro expression if necessary
    if ex.head === :macrocall
        mod !== nothing || Base.throw(ArgumentError("The `mod` keyword argument must be specified."))
        ex = macroexpand(mod, ex)
    end

    # ..........
end

@omus
Copy link
Contributor

omus commented Jul 12, 2020

You should be able to drop the mod requirement if you instead used __module__. Definitely worth a PR!

@tk3369
Copy link
Author

tk3369 commented Jul 13, 2020

Unfortunately, __module__ only works in macros but not functions.

@omus
Copy link
Contributor

omus commented Jul 13, 2020

My bad there. I was still thinking about the @foo and @bar macros. Can you provide more context with what the @foo and @bar macros are going to be doing? I'm trying to make ensure performing the macroexpand in splitdef is the right thing to do.

@tk3369
Copy link
Author

tk3369 commented Jul 13, 2020

I happened to build two macros for different packages (hence difference purposes) that generate functions using splitdef and combinedef. I'd like to be able to compose them, and the ordering or composition doesn't really matter.

Currently, I can make it work now by just adding a single line of code right before running splitdef(ex):

ex = ex.head === :macrocall ? macroexpand(__module__, ex) : ex

A more robust version can probably catch any exception and report back a nice error message. The issue is that it has to be added to both macros and so the logic is repeated....

To be honest, I am not sure if changing splitdef is the best path either. Another idea - what if we just add a new function in ExprTools called resolvedef (or whatever more appropriate) to do that work? I still need to call it before splitdef but at least logic is captured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants