Interpolation syntax in Threads.@spawn breaks $-aware macros

[tkf]

It seems interpolation syntax for @spawn #33119 is a breaking change:

julia> x = [reshape(1:4, 2, 2);]
2×2 Array{Int64,2}:
 1  3
 2  4

julia> @. exp(x)
2×2 Array{Float64,2}:
 2.71828  20.0855
 7.38906  54.5982

julia> @. $exp(x)
2×2 Array{Float64,2}:
 51.969   112.105
 74.7366  164.074

julia> fetch(Threads.@spawn @. $exp(x))
2×2 Array{Float64,2}:
 2.71828  20.0855
 7.38906  54.5982

julia> VERSION
v"1.4.0-DEV.634"

In Julia 1.3

julia> fetch(Threads.@spawn @. $exp(x))
2×2 Array{Float64,2}:
 51.969   112.105
 74.7366  164.074

But maybe it's OK to have breaking changes as @spawn is explicitly marked as experimental?

Stopping recursing into macrocall in the AST manipulation of @spawn can "fix" this problem. However, it's a bit limiting approach if you consider something like @spawn h(@spawn g($(f(x)))).

One approach may be to repeat $ to "escape" $; e.g., @spawn @. $$exp(x) if you want to do @. $exp(x) in another task. (This is of course breaking.)

[KristofferC]

Now add a @btime on top of that...

[JeffBezanson]

I think not recursing into other macro calls is sensible. For complex cases there is always the escape hatch of explicit let blocks.

[tkf]

cc @NHDaly

[NHDaly]

No, you're right. Not only is this a breaking change, it's also just broken. :(

Julia 1.2:

julia> fetch(@async @eval 2+$2)
4

Julia 1.4:

julia> fetch(@async @eval 2+$2)
ERROR: TaskFailedException:
UndefVarError: ##501 not defined

Alas. I guess my test coverage wasn't good enough. Sorry.
Unfortunately I'm on vacation now (merging while on vacation was probably a bad idea) but i'm trying to look at it now.

---

So i think this is just a bug in my parsing in the macro. My intention was that this would still parse exactly the same as before, and that the $ wouldn't be interpolating into the @spawn. It should be still treating it as part of the @eval. So then if you did want to interpolate into the spawn, you would need to use two $$. So this is just a bug.

I think if this is fixed, this should still be an okay change. My intention was that anywhere this is supported would previously have been a syntax: "$" expression outside quote error.

---

Stopping recursing into macrocall in the AST manipulation of @spawn can "fix" this problem. However, it's a bit limiting approach if you consider something like @spawn h(@spawn g($(f(x)))).

So I already added a mechanism to make sure not to interfere with any $ that are used to interpolate out of a quote or a string, so this works correctly:

julia> x = 2; fetch(Threads.@spawn "$x")
"2"

But it's not enough to just not recurse into strings/quotes, since I wanted to support "$$x" to capture the value into the @spawn. To get that to work, I basically tracked whether $ should be skipped because i'm inside a quote via a parameter passed to the recursion through the AST nodes.

However, i'm not sure if I want to do the same thing for macrocalls, because not all macros actually do make use of $.. For example, that would break wait(@async @info $x). And i'm pretty sure just not recursing into macrocalls would also break that.

(But yes, doing the same thing for macros does fix the broken @eval case I posted above, as well as fixes @tkf's original example!)

---

Would it perhaps be a reasonable solution to manually macroexpand any macrocalls I encounter while parsing, so that they can deal with the $ as they see fit, and THEN recurse into the expressions and if the $ is still there, I can treat it as an interpolation? I haven't seen that pattern used much in real code, but it seems like it most closely matches the behavior I want: Allow any inner macros to treat the $ interpolation however they want, and then treat any "extra" levels of interpolation as interpolation into the @spawn. Does that seem reasonable?

[NHDaly]

Hmm, so I tried doing that, calling macroexpand internally and then recursing into the resultant expression to replace one level of remaining $s, and that does work! It seems to fix all the examples we've discussed so far for macrocalls inside the @spawn!

julia> # Issue #34138
       @testset "spawn interpolation: inner macrocalls" begin
           x = 2
           @test fetch(@async @eval 2+$x) == 4
           @test fetch(@async @eval 2+$$x) == 4

           x = [reshape(1:4, 2, 2);]
           @test fetch(Threads.@spawn @. $exp(x)) == @. $exp(x)
       end
Test Summary:                         | Pass  Total
spawn interpolation: inner macrocalls |    3      3
Test.DefaultTestSet("spawn interpolation: inner macrocalls", Any[], 3, false)

HOWEVER it still doesn't work if there's a macro on the OUTSIDE:

julia> @eval fetch(@async 2+$$x)
ERROR: syntax: "$" expression outside quote
Stacktrace:
 [1] top-level scope at REPL[55]:1

julia> @eval fetch(@async @eval 2+$$x)
ERROR: syntax: "$" expression outside quote
Stacktrace:
 [1] top-level scope at REPL[56]:1

I guess this is because this "inside-out" evaluation strategy is not very common...?

[NHDaly]

I've just pushed up what i was trying here: #34148

Sorry i need to run to bed now. Hope some other good insight happens while I'm asleep! :)

[JeffBezanson]

Right; I believe that for multiple $ to work, all the involved macros need to support it. In fact the usual behavior is for the outermost macro that uses $ to capture them all, e.g.

julia> sym = :exp

julia> @eval (@. $sym([1,1]))
2-element Array{Float64,1}:
 2.718281828459045
 2.718281828459045

There clearly only @eval saw the $. So by that standard the current behavior of @spawn is ok. But we probably want spawn/async to interfere only minimally with the code it wraps, so it makes sense to just skip other macro calls.

[NHDaly]

There clearly only @eval saw the $. So by that standard the current behavior of @spawn is ok. But we probably want spawn/async to interfere only minimally with the code it wraps, so it makes sense to just skip other macro calls.

Mmm yeah, too bad. But you're right that we can always use an explicit let-block in those complex cases, as long as we make it clear how to do that.

---

As an aside, would it make sense to try to move everyone towards having macros all support multiple $, maybe by having them follow the pattern i presented above, to expand inner macros early? That's the kind of change we could do for 2.0, whenever that happens, yeah? Is that something that you'd find valuable?

[tkf]

@NHDaly Thanks for the quick patch PR and the thorough analysis!

would it make sense to try to move everyone towards having macros all support multiple $

I think imposing uniform meaning for $ is hard. For example, $ for @. is not really interpolation. Another example: PyCall.jl assigns a different meaning to $$ than $ in a string macro. I think it's conceivable that similar approach is used in some regular macros.