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

Interpreter/inference think the 3-argument `:method` Expr returns
`nothing`. Codegen thinks it returns the new method. I think the latter
makes more sense, because it lets us write explicit syntax-level
dependency links between method definitions and constants (used e.g. for
external abstract interpreters), which is something that Revise may need
in the future. Adjust the interpreter/inference to properly return the
method.

Author: Keno

Compiler/src/abstractinterpretation.jl

@@ -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)

src/interpreter.c

@@ -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
@@ -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);

src/julia-syntax.scm

@@ -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)
@@ -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))))
@@ -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)
@@ -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
@@ -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)))))

test/core.jl

@@ -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

test/syntax.jl

@@ -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