Open florius0 opened 1 week ago
Yup definitely a bug in decorator. If you check out the main branch atm you should have access to an Sibyl.Experimental
module that you can use
.
This replaces decorator
with something admittedly grosser, but it circumvents this error.
It works by overriding the def
macro so that we can define functions thusly:
def my_fun do
... sibyl code to emit event start ...
super() # actually whatever was passed into the `def` macro, but its the same idea
after
... sibyl error event emission ...
catch
... sibyl catch event emission ...
end
I admit that I'm unsure if this works OOTB with defoverridable
though.
Although now that I think about it... Sibyl.Experimental
could instead:
defoverridable
clause for all public functions in the moduleI believe if you have defoverridable
multiple times in a module it works as expected, super()
usage works as expected, etc.
Might be a less invasive approach?
But then I'm not sure how defoverridable works in tandem with functions with multiple clauses plus default args...
It works by overriding the def macro so that we can define functions thusly
I would say that is actually more sane than decorator's approach
Thx, I'll try it out shortly
I was able to fix defoverridable/1
by doing this:
defmodule Helpers.Sibyl do
defmacro __using__(:decorate_all) do
quote do
@before_compile unquote(__MODULE__)
end
end
defmacro __before_compile__(_env) do
quote do
use Sibyl
@decorate_all {Sibyl.Decorator, :trace, []}
end
end
end
note the expanded trace/0
in the attribute.
It works by overriding the def macro so that we can define functions thusly
It would fail to decorate custom def's.
Sibyl.Experimental could instead:
- inject a defoverridable clause for all public functions in the module
- for all public functions inject my code block as above I believe if you have defoverridable multiple times in a module it works as expected, super() usage works as expected, etc.
Might be a less invasive approach?
This approach might be better, but it would restrict its use to def/2
only.
Two alternative approaches were revealed to me (in a dream):
.beam
s directly, the drawback would be that we would essentially be operating in erlang-land, so no macros and custom def
s.Could you elaborate on what you mean by "custom def"?
e.g. the module I have in my project for defining runtime-configurable defdelegate
s
defmodule DI do
defmacro __using__(opts) do
quote location: :keep, generated: true do
import unquote(__MODULE__)
@app unquote(opts)[:app] || Mix.Project.config()[:app]
@path unquote(opts)[:path] || [__MODULE__, :impl]
def impl, do: Utils.Application.fetch_env!(@app, @path)
defoverridable impl: 0
end
end
defmacro defdi({name, _meta, args}) do
vars =
Macro.prewalk(args, fn
{:\\, _meta, [var, _default]} -> var
ast -> ast
end)
quote do
def unquote({name, [line: __ENV__.line], args})
def unquote({name, [line: __ENV__.line], vars}), do: impl().unquote(name)(unquote_splicing(vars))
end
end
end
I was able to fix defoverridable/1 by doing this:
UPD: it didn't work
Current iteration of Sibyl.Experimental
doesn't work when use
d inside a macro (weird, IMO it should).
I managed to fix all of my problems with decorator tho.
...defines defaults multiple times. Elixir allows defaults to be declared once per definition
– turns out, @on_definition
is being called multiple times with the exact same args, so doing Enum.uniq/1
fixes it.defoverridable/1
stops working - and this happens bcs of how decorator works. It thinks that overrided function is a clause of previously defined one. I had to make assumption that clauses are always together. Potential bug is if it would instantly override the function after its definition, that's what Decorator.Separator
is for.# lib/decorator/decorate.ex
defmacro before_compile(env) do
decorated =
env.module
|> Module.get_attribute(:decorated)
|> Enum.uniq()
|> Enum.reverse()
PS I just realised that my fix would break if function has it's caluse defined in separate places, e.g.
defmodule A do
defmacro __using__(_opts) do
quote do
def f(0), do: :zero
end
end
end
defmodule B do
use Sibyl
@decorate_all trace()
use A
def f(x), do: x
end
B.f(0) #=> :zero
Could be fixed later with defoverridable/1
, but would require one to look up original clauses.
I'm in favour of this behaviour compared to existing one, which couldn't be fixed in user code at all
IMO we should mention this problem in readme.
Unrelated, but it would be nice:
@decorate_all
I wouldn't want to trace smth like __schema__/*
. And I suspect it has impact on performanceSibyl.Event.reflect/*
.Maybe I'll implement both today.
UPD
- To somehow cache and ideally pre-fill
Sibyl.Event.reflect/*
Is not so necessary, since all modules in releases are loaded at startup, thus reducing Sibyl.Event.reflect/*
time considerably
When including Sibyl into other
__using__/1
macros, comilation breaks in several ways:...defines defaults multiple times. Elixir allows defaults to be declared once per definition
defoverridable/1
stops working.Example:
PS
defoverridable/1
can be fixed ifuse Sibyl...
is declared after it. Perhaps decoration can be done as the last step of module compilation? PPS looks like it's Decorator bug, not Sibyl's