Skip to content
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

Make sure :push_loc meta always has a corresponding :pop_loc #18180

Merged
merged 1 commit into from
Aug 26, 2016
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
15 changes: 5 additions & 10 deletions base/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/ast.scm
Original file line number Diff line number Diff line change
Expand Up @@ -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)))))
Expand Down
35 changes: 27 additions & 8 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
107 changes: 107 additions & 0 deletions test/parse.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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