From b022c1975ba2f4b5b38a1d178f50203f07cd9015 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 20 Jun 2024 05:40:43 +0000 Subject: [PATCH] Follow up #54772 - don't accidentally put `Module` into method name slot 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. --- src/julia-syntax.scm | 8 +++----- test/syntax.jl | 13 +++++++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index 5a50bb0cba41a8..8454c58d3f8e23 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -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) @@ -4056,7 +4054,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)))) diff --git a/test/syntax.jl b/test/syntax.jl index 3c0b8db78a5600..4a370dbd797f30 100644 --- a/test/syntax.jl +++ b/test/syntax.jl @@ -3747,3 +3747,16 @@ b54805 = 2 end using .Export54805 @test b54805 == 2 + +# 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