Is there a better way to write this macro?

The whole string/eval thing is definitely not the recommended way to do it. There’s lots of ways to write this, but my preferred way would be the following:

macro dispatchinstruction(inst, vm)
    let vm = esc(vm), inst = esc(inst)
        foldr(instances(Opcode); init=:(error("Illegal value"))) do opcode, else_branch
            cond = :($inst.op == $opcode)
            fname = Symbol(:on, opcode)
            body = :($fname($inst, $vm))
            Expr(:if, cond, body, else_branch)
        end
    end
end

The important changes I made were

  • inst and vm are arguments to the macro, rather than implicitly assuming that the caller has variables defined in their local scope named inst and vm.
  • No use of string metaprogramming, we instead build up a series of if statements by folding over them
  • No use of eval.
  • Don’t esc the function fname since that should be defined in the same namespace that this macro and Opcode were defined in, not necessarily in the module where the macro is called.

Here’s the generated code:

julia> @enum Opcode a b c d

julia> @macroexpand1 @dispatchinstruction(inst, vm)
:(if inst.op == a
      Main.ona(inst, vm)
  else
      if inst.op == b
          Main.onb(inst, vm)
      else
          if inst.op == c
              Main.onc(inst, vm)
          else
              if inst.op == d
                  Main.ond(inst, vm)
              else
                  Main.error("Illegal value")
              end
          end
      end
  end)

Note that the Mains you see in the above code will become whatever module the macro was defined in, not whatever module the macro was called in, since julia macros are hygenic by default.

7 Likes