Pass expr_compat_mode into the new @eval
#113
Conversation
c42f
left a comment
c42f
left a comment
There was a problem hiding this comment.
I find it worrying that the @eval implementation itself needs to know about expr_compat_mode here.
I think it's ok to make @eval somewhat special as a stopgap but users may define their own @eval-like macros which makes it tricky to deal with the general problem. It seems that "should a quoted expression be an Expr or SyntaxTree?" isn't straightforward in general.
| let T = :foo | ||
| @eval @doc $"This is a $T" $T = 1 | ||
| end | ||
| """; expr_compat_mode=true) === 1 |
There was a problem hiding this comment.
It's not clear what this is testing - can we be more precise here? And hopefully avoid Base.@doc in a unit test - it's very complicated 😅
Is the point that the type of @eval(quote x end) should be Expr when expr_compat_mode==true and SyntaxTree otherwise?
There was a problem hiding this comment.
It is complicated, but we have no integration tests yet and I'd like to prevent @doc from biting me again in the stdlib war (this case is reduced from LibGit2/repository.jl). I can add a simpler test above it though.
2 participants
A small tweak to the macro added in #107 to unbreak stdlibs. Looks a bit strange, but life is good when there's only one sensible way of doing it.