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

@@ -2042,15 +2042,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
@@ -2081,7 +2075,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