Skip to content

Commit

Permalink
Follow up #54772 - don't accidentally put Module into method name s…
Browse files Browse the repository at this point in the history
…lot (#54856)

The `:outerref` removal (#54772) ended up accidentally putting a
`Module` argument into 3-argument `:method` due to a bad refactor. We
didn't catch this in the tests, because the name slot of 3-argument
`:method` is
 unused (except for external method tables), but ordinarily contains
the same data as 1-argument `:method`.
That said, some packages in the Revise universe look at this (arguably
incorrectly, since they should be looking at the signature instead), so
it should be correct until we fix Revise, at which point we may just
want to always pass `false` here.
  • Loading branch information
Keno committed Jun 23, 2024
1 parent dfd1d49 commit 696d9c3
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
8 changes: 3 additions & 5 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,8 @@

(define (method-expr-name m)
(let ((name (cadr m)))
(let ((name (if (or (length= m 2) (not (pair? name)) (not (quoted? name))) name (cadr name))))
(cond ((not (pair? name)) name)
((eq? (car name) 'globalref) (caddr name))
(else name)))))
(cond ((globalref? name) (caddr name))
(else name))))

;; extract static parameter names from a (method ...) expression
(define (method-expr-static-parameters m)
Expand Down Expand Up @@ -4087,7 +4085,7 @@ f(x) = yt(x)
(newlam (compact-and-renumber (linearize (car exprs)) 'none 0)))
`(toplevel-butfirst
(block ,@sp-inits
(method ,name ,(cl-convert sig fname lam namemap defined toplevel interp opaq globals locals)
(method ,(cadr e) ,(cl-convert sig fname lam namemap defined toplevel interp opaq globals locals)
,(julia-bq-macro newlam)))
,@top-stmts))))

Expand Down
13 changes: 13 additions & 0 deletions test/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3845,3 +3845,16 @@ struct SingletonMaker; end
const no_really_this_is_a_function_i_promise = Val{SingletonMaker()}()
no_really_this_is_a_function_i_promise(a) = 2 + a
@test Val{SingletonMaker()}()(2) == 4

# Test that lowering doesn't accidentally put a `Module` in the Method name slot
let src = @Meta.lower let capture=1
global foo_lower_block
foo_lower_block() = capture
end
code = src.args[1].code
for i = length(code):-1:1
expr = code[i]
Meta.isexpr(expr, :method) || continue
@test isa(expr.args[1], Union{GlobalRef, Symbol})
end
end

0 comments on commit 696d9c3

Please sign in to comment.