Skip to content

Align :method Expr return value between interpreter and codegen #58076

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

Merged
merged 2 commits into from
Apr 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Compiler/src/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3409,7 +3409,7 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, ssta
elseif ehead === :cfunction
return abstract_eval_cfunction(interp, e, sstate, sv)
elseif ehead === :method
rt = (length(e.args) == 1) ? Any : Nothing
rt = (length(e.args) == 1) ? Any : Method
return RTEffects(rt, Any, EFFECTS_UNKNOWN)
elseif ehead === :copyast
return abstract_eval_copyast(interp, e, sstate, sv)
Expand Down
7 changes: 4 additions & 3 deletions src/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ static jl_value_t *eval_methoddef(jl_expr_t *ex, interpreter_state *s)
}
atypes = eval_value(args[1], s);
meth = eval_value(args[2], s);
jl_method_def((jl_svec_t*)atypes, mt, (jl_code_info_t*)meth, s->module);
jl_method_t *ret = jl_method_def((jl_svec_t*)atypes, mt, (jl_code_info_t*)meth, s->module);
JL_GC_POP();
return jl_nothing;
return (jl_value_t *)ret;
}

// expression evaluator
Expand Down Expand Up @@ -626,7 +626,8 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
}
else if (toplevel) {
if (head == jl_method_sym && jl_expr_nargs(stmt) > 1) {
eval_methoddef((jl_expr_t*)stmt, s);
jl_value_t *res = eval_methoddef((jl_expr_t*)stmt, s);
s->locals[jl_source_nslots(s->src) + s->ip] = res;
}
else if (head == jl_toplevel_sym) {
jl_value_t *res = jl_toplevel_eval(s->module, stmt);
Expand Down
29 changes: 18 additions & 11 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -439,9 +439,11 @@
,body))))
(if (or (symbol? name) (globalref? name))
`(block ,@generator (method ,name) (latestworld-if-toplevel) ,mdef (unnecessary ,name)) ;; return the function
(if (not (null? generator))
`(block ,@generator ,mdef)
mdef))))))
(if (overlay? name)
(if (not (null? generator))
`(block ,@generator ,mdef)
mdef)
`(block ,@generator ,mdef (null))))))))

;; wrap expr in nested scopes assigning names to vals
(define (scopenest names vals expr)
Expand Down Expand Up @@ -4127,6 +4129,7 @@ f(x) = yt(x)
'()
(map-cl-convert (butlast (cdr sig))
fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)))
(r (make-ssavalue))
(sig (and sig (if (eq? (car sig) 'block)
(last sig)
sig))))
Expand All @@ -4150,7 +4153,7 @@ f(x) = yt(x)
((null? cvs)
`(block
,@sp-inits
(method ,(cadr e) ,(cl-convert
(= ,r (method ,(cadr e) ,(cl-convert
;; anonymous functions with keyword args generate global
;; functions that refer to the type of a local function
(rename-sig-types sig namemap)
Expand All @@ -4162,17 +4165,19 @@ f(x) = yt(x)
`(lambda ,(cadr lam2)
(,(clear-capture-bits (car vis))
,@(cdr vis))
,body)))
(latestworld)))
,body))))
(latestworld)
,r))
(else
(let* ((exprs (lift-toplevel (convert-lambda lam2 '|#anon| #t '() #f parsed-method-stack)))
(top-stmts (cdr exprs))
(newlam (compact-and-renumber (linearize (car exprs)) 'none 0)))
`(toplevel-butfirst
(block ,@sp-inits
(method ,(cadr e) ,(cl-convert sig fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)
,(julia-bq-macro newlam))
(latestworld))
(= ,r (method ,(cadr e) ,(cl-convert sig fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)
,(julia-bq-macro newlam)))
(latestworld)
,r)
,@top-stmts))))

;; local case - lift to a new type at top level
Expand Down Expand Up @@ -4999,8 +5004,10 @@ f(x) = yt(x)
(let ((l (make-ssavalue)))
(emit `(= ,l ,(compile lam break-labels #t #f)))
l))))
(emit `(method ,(or (cadr e) '(false)) ,sig ,lam))
(if value (compile '(null) break-labels value tail)))
(let ((val (make-ssavalue)))
(emit `(= ,val (method ,(or (cadr e) '(false)) ,sig ,lam)))
(if tail (emit-return tail val))
val))
(cond (tail (emit-return tail e))
(value e)
(else (emit e)))))
Expand Down
5 changes: 3 additions & 2 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -8308,11 +8308,12 @@ end
module OverlayModule

using Base.Experimental: @MethodTable, @overlay
using Test

@MethodTable mt
# long function def
@overlay mt function sin(x::Float64)
1
let m = @overlay mt function sin(x::Float64); 1; end
@test isa(m, Method)
end
# short function def
@overlay mt cos(x::Float64) = 2
Expand Down
8 changes: 8 additions & 0 deletions test/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4315,3 +4315,11 @@ module DoubleImport
import Random
end
@test DoubleImport.Random === Test.Random

# Expr(:method) returns the method
let ex = @Meta.lower function return_my_method(); 1; end
code = ex.args[1].code
idx = findfirst(ex->Meta.isexpr(ex, :method) && length(ex.args) > 1, code)
code[end] = Core.ReturnNode(Core.SSAValue(idx))
@test isa(Core.eval(@__MODULE__, ex), Method)
end