-
Notifications
You must be signed in to change notification settings - Fork 157
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
@from_network
does more strange eval
stuff
#703
Comments
As I understand it, all macros ultimately call Perhaps you can be more specific about the strange, dubious and scary behaviour you are experiencing. I have not experienced any problems except when the macro is called in a baremodule. |
Doing Consider this example: macro crazy()
Base.eval(__module__, :(struct Crazy end))
end
macro sane()
esc(:(struct Sane end))
end
function testcrazy()
@crazy
nothing
end then julia> testcrazy()
julia> Crazy()
Crazy()
julia> sane() = (@sane(); nothing)
ERROR: syntax: "struct" expression not at top level
Stacktrace:
[1] top-level scope at REPL[3]:1
julia> @macroexpand @crazy() # note that if `Crazy` were not already defined, this would have defined it unexpectedly
julia> @macroexpand @sane()
:(struct Sane
#= /home/expandingman/src/scrap.jl:7 =#
end) Based on my current understanding of MLJ, I do not believe |
Btw, I'm looking into the code a little bit as I may be able to help out, but it seems that there are |
Okay, I see how we have not followed best practice here. Really appreciate the feedback and guidance. It's hard for me to imagine living without these evaluations as a lot of logic depends on them. In retrospect, if we weren't constructing new types, we might have avoided the meta-programming altogether - see #594 (comment) - but I think that is a complete redesign. I guess in principle all this logic could be shifted into the final expression - but it would be a bit of work.
julia> @time using MacroTools
0.389238 seconds (1.18 M allocations: 58.403 MiB, 5.23% gc time) Sure. I should take a look. |
Yeah, I'm not yet as convinced that If it really is the case that "a lot of logic depends on" the For example macro from_network(args...)
expr = Expr(:block)
# replace
# eval(ex1) with
push!(expr.args, ex1)
# more stuff
esc(expr)
end Though I haven't spent much time on it yet, so there may be some major things missing which can't be easily written this way. |
Appreciate the code review and help! |
I also just realized that although |
So apparently
@load
(thanks to @ablaom for fixing!) wasn't the only macro with dubious and scary behavior.@from_network
also evals stuff directly into the module. Again, this is extremely unexpected behavior for the user, this is just not what macros normally do. It would be great if this could be fixed as well.The text was updated successfully, but these errors were encountered: