Skip to content

Using @sync from a package macro fails when the same result works in the REPL #28775

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

Open
ChrisRackauckas opened this issue Aug 20, 2018 · 5 comments
Labels
bug Indicates an unexpected problem or unintended behavior compiler:lowering Syntax lowering (compiler front end, 2nd stage) macros @macros

Comments

@ChrisRackauckas
Copy link
Member

Maybe this is simpler than I think, but ChrisRackauckas/ParallelDataTransfer.jl#14 seems to demonstrate that using @sync from outside the Main module gives an issue with let. MWE:

module Tester
  macro tester()
    quote
      @sync for i in 1:5
        println(i)
      end
    end
  end
end
using Main.Tester
Tester.@tester

throws

syntax: invalid let syntax
include_string(::Module, ::String, ::String) at loading.jl:1002
include_string(::Module, ::String, ::String, ::Int64) at eval.jl:30
(::getfield(Atom, Symbol("##110#115")){String,Int64,String})() at eval.jl:89
withpath(::getfield(Atom, Symbol("##110#115")){String,Int64,String}, ::String) at utils.jl:30
withpath at eval.jl:46 [inlined]
#109 at eval.jl:88 [inlined]
hideprompt(::getfield(Atom, Symbol("##109#114")){String,Int64,String}) at repl.jl:76
macro expansion at eval.jl:87 [inlined]
(::getfield(Atom, Symbol("##108#113")){Dict{String,Any}})() at task.jl:85
@andreasnoack
Copy link
Member

andreasnoack commented Aug 20, 2018

It looks like an issue with the escape in

julia/base/task.jl

Lines 241 to 243 in f6c48eb

var = esc(sync_varname)
quote
let $var = Any[]
For some reason there is some module information prepended when the macro gets expanded

julia> macroexpand(Main, :(Tester.@tester()))
quote
    #= REPL[8]:4 =#
    begin
        #= task.jl:243 =#
        let Main.Tester.:(##sync#72) = (Base.Any)[]
            #= task.jl:244 =#
            #12#v = for #11#i = 1:5
                    #= REPL[8]:5 =#
                    (Main.Tester.println)(#11#i)
                end
            #= task.jl:245 =#
            (Base.sync_end)(Main.Tester.:(##sync#72))
            #= task.jl:246 =#
            #12#v
        end
    end
end

@ChrisRackauckas
Copy link
Member Author

Note that this is a regression from v0.6: the broadcast code from which this was found used to work.

@KristofferC
Copy link
Member

KristofferC commented Aug 20, 2018

Hygiene for macros inside macros feels impossible to do correctly now. Perhaps it was impossible before as well, but it seemed to generally work as you expected. Ref #22985

@tkf
Copy link
Member

tkf commented Feb 4, 2020

I had a similar problem (1.5.0-DEV). The only solution I've found was to aggressively use interpolation:

julia> macro sync_wrapper_hygiene(ex)
           quote
               @sync begin
                   $(esc(ex))
               end
           end
       end
@sync_wrapper_hygiene (macro with 1 method)

julia> macro sync_wrapper_interpolation(ex)
           quote
               $Base.@sync begin
                   $ex
               end
           end |> esc
       end
@sync_wrapper_interpolation (macro with 1 method)

julia> @sync_wrapper_hygiene nothing
ERROR: syntax: invalid let syntax
Stacktrace:
 [1] top-level scope at REPL[3]:1

julia> @sync_wrapper_interpolation nothing  # works

(It breaks even within REPL, though.)

Related? #20241 #23221

@vtjnash vtjnash added compiler:lowering Syntax lowering (compiler front end, 2nd stage) macros @macros bug Indicates an unexpected problem or unintended behavior labels Sep 15, 2020
@vtjnash
Copy link
Member

vtjnash commented Sep 15, 2020

This is likely just a bug in the macroexpand.scm code. It's got a buggy re-implementation of resolve-scopes, and it is simply unaware of how to handle escape (its default when it doesn't understand some syntax is to ignore it). That works okay when it's the outermost scope, but fails well when it's nested. This is a particularly complicated case too, since this macro is intentionally wanting to leak this variable into the enclosed code (possibly a case where #6910 would be much better?), as sync_var is supposed to have the same count of escape calls wrapping it as the body argument is going to eventually have.

julia> @macroexpand1 Tester.@tester
quote
    #= REPL[7]:4 =#
    #= REPL[7]:4 =# @sync for i = 1:5
            #= REPL[7]:5 =#
            println(i)
        end
end

julia> Meta.macroexpand(Main, Expr(:var"hygienic-scope", ans, Main), recursive=false)
quote
    #= REPL[7]:4 =#
    begin
        #= task.jl:367 =#
        let Main.:(var"##sync#33") = Base.Channel(Base.Inf)
            #= task.jl:368 =#
            var"#19#v" = for var"#18#i" = 1:5
                    #= REPL[7]:5 =#
                    Main.println(var"#18#i")
                end
            #= task.jl:369 =#
            Base.sync_end(Main.:(var"##sync#33"))
            #= task.jl:370 =#
            var"#19#v"
        end
    end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior compiler:lowering Syntax lowering (compiler front end, 2nd stage) macros @macros
Projects
None yet
Development

No branches or pull requests

5 participants