Skip to content
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

@inbounds overrides val variable in macros. Wrong usage of local #54417

Open
0x0f0f0f opened this issue May 8, 2024 · 4 comments
Open

@inbounds overrides val variable in macros. Wrong usage of local #54417

0x0f0f0f opened this issue May 8, 2024 · 4 comments
Labels
macros @macros

Comments

@0x0f0f0f
Copy link

0x0f0f0f commented May 8, 2024

It took me a while when debugging a compiled pattern symbolics matcher to realize why pattern variables i've named val were overridden by other values. I've ended up adding a prefix to those symbols to avoid the issue. The macro shouldn't be escaped, as fresh symbols are needed to avoid clashes.

julia> macro foo(x,i)
           v = [1,2,3,4,5]
           quote
               val = $x
               t = @inbounds ($v)[$i]
               println(val)
           end
       end

julia> @foo(2,3)
3

The reason is because

macro inbounds(blk)
    return Expr(:block,
        Expr(:inbounds, true),
        Expr(:local, Expr(:(=), :val, esc(blk))),
        Expr(:inbounds, :pop),
        :val)
end

My intuition was always that nested macro calls that "access" the same symbol, should hygiene the symbol in the innermost macro expansion, but it seems it's not the case. What's the correct pattern of macro hygiene for these cases? shall I file a PR renaming that val in @inbounds to something less likely to clash with outer identifiers?

julia> @macroexpand @foo(2,3)
quote
    #= REPL[39]:4 =#
    var"#9332#val" = 2
    #= REPL[39]:5 =#
    var"#9333#t" = begin
            $(Expr(:inbounds, true))
            local var"#9332#val" = ([1, 2, 3, 4, 5])[3]
            $(Expr(:inbounds, :pop))
            var"#9332#val"
        end
    #= REPL[39]:6 =#
    Main.println(var"#9332#val")
end
julia> versioninfo()
Julia Version 1.10.2
Commit bd47eca2c8a (2024-03-01 10:14 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 8 × 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, tigerlake)
Threads: 1 default, 0 interactive, 1 GC (on 8 virtual cores)
Environment:
  JULIA_EDITOR = code
  JULIA_NUM_THREADS = 

Installed via juliaup

@vtjnash
Copy link
Sponsor Member

vtjnash commented May 9, 2024

This sounds like a bug nearly identical to #53667, but yeah we could rename that to something more awkward to avoid clashes while this bug is still pending in the hygiene computation

@Pangoraw
Copy link
Contributor

Pangoraw commented May 9, 2024

Linking #48910 which has a similar cause/problem (although with @inline).

@0x0f0f0f
Copy link
Author

0x0f0f0f commented May 9, 2024

I'm confused by the behavior of local?

julia> begin 
           x = 2 
           begin 
               local x = 3
           end
           x
       end
3

julia> x = 2 
           begin 
               local x = 3
           end
           x
2 # works correctly in global scope? 

julia> begin 
           x = 2 
           if x > 1
               local x = 3
           end
           x
       end
3

julia> begin 
           x = 2 
           for i in 1:x
               local x = 3
           end
           x
       end
2

I'm referring to the manual page on scoping https://docs.julialang.org/en/v1/manual/variables-and-scoping/

Notably missing from this table are begin blocks and if blocks which do not introduce new scopes. The three types of scopes follow somewhat different rules which will be explained below.

If local has no effect on begin and if blocks, why use it in @inbounds and @inline?

@0x0f0f0f 0x0f0f0f changed the title @inbounds overrides val variable in macros @inbounds overrides val variable in macros. Wrong usage of local May 9, 2024
@giordano giordano added the macros @macros label May 9, 2024
@giordano
Copy link
Contributor

giordano commented May 9, 2024

Also similar to #41451 and #48015 (for logging macros).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
macros @macros
Projects
None yet
Development

No branches or pull requests

4 participants