Make sure :push_loc meta always has a corresponding :pop_loc

* Preserve meta node and line number info during dead code elimination
* Insert `:push_loc` and `:pop_loc` in pairs during lowering

Fix #16578

base/inference.jl

@@ -2041,15 +2041,9 @@ function eval_annotate(e::ANY, vtypes::ANY, sv::InferenceState, undefs, pass)
 end
 
 function expr_cannot_delete(ex::Expr)
-    # This alone should be enough for any sane use of
-    # `Expr(:inbounds)` and `Expr(:boundscheck)`. However, it is still possible
-    # to have these embeded in other expressions (e.g. `return @inbounds ...`)
-    # so we check recursively if there's a matching expression
-    (ex.head === :inbounds || ex.head === :boundscheck) && return true
-    for arg in ex.args
-        isa(arg, Expr) && expr_cannot_delete(arg::Expr) && return true
-    end
-    return false
+    head = ex.head
+    return (head === :inbounds || head === :boundscheck || head === :meta ||
+            head === :line)
 end
 
 # annotate types of all symbols in AST
@@ -2080,7 +2074,8 @@ function type_annotate!(linfo::LambdaInfo, states::Array{Any,1}, sv::ANY, nargs)
                 record_slot_type!(id, widenconst(states[i+1][id].typ), linfo.slottypes)
             end
         elseif optimize
-            if isa(expr, Expr) && expr_cannot_delete(expr::Expr)
+            if ((isa(expr, Expr) && expr_cannot_delete(expr::Expr)) ||
+                isa(expr, LineNumberNode))
                 i += 1
                 continue
             end

src/ast.scm

@@ -211,6 +211,10 @@
 (define (make-assignment l r) `(= ,l ,r))
 (define (assignment? e) (and (pair? e) (eq? (car e) '=)))
 (define (return? e) (and (pair? e) (eq? (car e) 'return)))
+(define (complex-return? e) (and (return? e)
+                                 (let ((x (cadr e)))
+                                   (not (or (simple-atom? x) (ssavalue? x)
+                                            (equal? x '(null)))))))
 
 (define (eq-sym? a b)
   (or (eq? a b) (and (ssavalue? a) (ssavalue? b) (eqv? (cdr a) (cdr b)))))

src/julia-syntax.scm

@@ -3184,20 +3184,39 @@ f(x) = yt(x)
                     (fname      (if (and (length> e 1) (pair? fnm) (eq? (car fnm) 'line)
                                          (length> fnm 2))
                                     (caddr fnm)
-                                    filename)))
-               (if (not (eq? fname last-fname))
-                   (begin (set! filename fname)
-                          ;; don't need a filename node for start of function
-                          (if (not (eq? e (lam:body lam))) (emit `(meta push_loc ,fname)))))
+                                    filename))
+                    (file-diff  (not (eq? fname last-fname)))
+                    ;; don't need a filename node for start of function
+                    (need-meta  (and file-diff
+                                     (not (eq? e (lam:body lam))))))
+               (if file-diff (set! filename fname))
+               (if need-meta (emit `(meta push_loc ,fname)))
                (begin0
                 (let loop ((xs (cdr e)))
                   (if (null? (cdr xs))
                       (compile (car xs) break-labels value tail)
                       (begin (compile (car xs) break-labels #f #f)
                              (loop (cdr xs)))))
-                (if (not (eq? fname last-fname))
-                    (begin (if (not tail) (emit `(meta pop_loc)))
-                           (set! filename last-fname))))))
+                (if need-meta
+                    (if (or (not tail)
+                            (and (pair? (car code))
+                                 (or (eq? (cdar code) 'meta)
+                                     (eq? (cdar code) 'line))))
+                        (emit '(meta pop_loc))
+                        ;; If we need to return the last non-meta expression
+                        ;; splice the pop before the result
+                        (let ((retv (car code))
+                              (body (cdr code)))
+                          (set! code body)
+                          (if (complex-return? retv)
+                              (let ((tmp (make-ssavalue)))
+                                (emit `(= ,tmp ,(cadr retv)))
+                                (emit '(meta pop_loc))
+                                (emit `(return ,tmp)))
+                              (begin
+                                (emit '(meta pop_loc))
+                                (emit retv))))))
+                (if file-diff (set! filename last-fname)))))
             ((return)
              (compile (cadr e) break-labels #t #t)
              '(null))

test/parse.jl

@@ -670,3 +670,110 @@ end
 
 # issue 15896 and PR 15913
 @test_throws ErrorException eval(:(macro test15896(d; y=0) end))
+
+# Issue #16578 (Lowering) mismatch between push_loc and pop_loc
+module TestMeta_16578
+using Base.Test
+function get_expr_list(ex)
+    if isa(ex, LambdaInfo)
+        return Base.uncompressed_ast(ex)
+    elseif ex.head == :thunk
+        return get_expr_list(ex.args[1])
+    else
+        return ex.args
+    end
+end
+
+function count_meta_loc(exprs)
+    push_count = 0
+    pop_count = 0
+    for expr in exprs
+        Meta.isexpr(expr, :meta) || continue
+        expr = expr::Expr
+        if expr.args[1] === :push_loc
+            push_count += 1
+        elseif expr.args[1] === :pop_loc
+            pop_count += 1
+        end
+        @test push_count >= pop_count
+    end
+    @test push_count == pop_count
+    return push_count
+end
+
+function is_return_ssavalue(ex::Expr)
+    ex.head === :return && isa(ex.args[1], SSAValue)
+end
+
+function is_pop_loc(ex::Expr)
+    ex.head === :meta && ex.args[1] === :pop_loc
+end
+
+# Macros
+macro m1()
+    quote
+        sin(1)
+    end
+end
+macro m2()
+    quote
+        1
+    end
+end
+include_string("""
+macro m3()
+    quote
+        @m1
+    end
+end
+macro m4()
+    quote
+        @m2
+    end
+end
+""", "another_file.jl")
+m1_exprs = get_expr_list(expand(:(@m1)))
+m2_exprs = get_expr_list(expand(:(@m2)))
+m3_exprs = get_expr_list(expand(:(@m3)))
+m4_exprs = get_expr_list(expand(:(@m4)))
+
+# Check the expanded expresion has expected number of matching push/pop
+# and the return is handled correctly
+@test count_meta_loc(m1_exprs) == 1
+@test is_return_ssavalue(m1_exprs[end])
+@test is_pop_loc(m1_exprs[end - 1])
+
+@test count_meta_loc(m2_exprs) == 1
+@test m2_exprs[end] == :(return 1)
+@test is_pop_loc(m2_exprs[end - 1])
+
+@test count_meta_loc(m3_exprs) == 2
+@test is_return_ssavalue(m3_exprs[end])
+@test is_pop_loc(m3_exprs[end - 1])
+
+@test count_meta_loc(m4_exprs) == 2
+@test m4_exprs[end] == :(return 1)
+@test is_pop_loc(m4_exprs[end - 1])
+
+function f1(a)
+    b = a + 100
+    b
+end
+
+@generated function f2(a)
+    quote
+        b = a + 100
+        b
+    end
+end
+
+f1_exprs = get_expr_list(@code_typed f1(1))
+@test count_meta_loc(f1_exprs) == 0
+@test Meta.isexpr(f1_exprs[end], :return)
+
+f2_exprs = get_expr_list(@code_typed f2(1))
+@test count_meta_loc(f2_exprs) == 1
+@test is_pop_loc(f2_exprs[end - 1])
+@test Meta.isexpr(f2_exprs[end], :return)
+
+end