Skip to content

Commit

Permalink
fix #26743, spurious return path in try-finally in tail position
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson committed Apr 9, 2018
1 parent b08c262 commit 43876e3
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 25 deletions.
56 changes: 31 additions & 25 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -3460,17 +3460,18 @@ f(x) = yt(x)
(tmp (if (valid-ir-return? x) #f (make-ssavalue))))
(if tmp (emit `(= ,tmp ,x)))
(emit `(return ,(or tmp x)))))
(if (> handler-level 0)
(let ((tmp (cond ((and (simple-atom? x) (or (not (ssavalue? x)) (not finally-handler))) #f)
(finally-handler (new-mutable-var))
(else (make-ssavalue)))))
(if tmp (emit `(= ,tmp ,x)))
(if finally-handler
(leave-finally-block `(return ,(or tmp x)))
(begin (emit `(leave ,handler-level))
(actually-return (or tmp x))))
(or tmp x))
(actually-return x)))
(if x
(if (> handler-level 0)
(let ((tmp (cond ((and (simple-atom? x) (or (not (ssavalue? x)) (not finally-handler))) #f)
(finally-handler (new-mutable-var))
(else (make-ssavalue)))))
(if tmp (emit `(= ,tmp ,x)))
(if finally-handler
(leave-finally-block `(return ,(or tmp x)))
(begin (emit `(leave ,handler-level))
(actually-return (or tmp x))))
(or tmp x))
(actually-return x))))
(define (emit-break labl)
(let ((lvl (caddr labl)))
(if (and finally-handler (> (cadddr finally-handler) lvl))
Expand Down Expand Up @@ -3512,7 +3513,9 @@ f(x) = yt(x)
(if (null? lst)
(reverse! vals)
(let* ((arg (car lst))
(aval (compile arg break-labels #t #f linearize)))
(aval (or (compile arg break-labels #t #f linearize)
;; TODO: argument exprs that don't yield a value?
'(null))))
(loop (cdr lst)
(cons (if (and temps? linearize (not simple?)
(not (simple-atom? arg))
Expand All @@ -3530,20 +3533,23 @@ f(x) = yt(x)
aval)
vals))))))))
(define (compile-cond ex break-labels)
(let ((cnd (compile ex break-labels #t #f)))
(let ((cnd (or (compile ex break-labels #t #f)
;; TODO: condition exprs that don't yield a value?
'(null))))
(if (and *very-linear-mode*
(not (valid-ir-argument? cnd)))
(let ((tmp (make-ssavalue)))
(emit `(= ,tmp ,cnd))
tmp)
cnd)))
(define (emit-assignment lhs rhs)
(if (valid-ir-rvalue? lhs rhs)
(emit `(= ,lhs ,rhs))
(let ((rr (make-ssavalue)))
(emit `(= ,rr ,rhs))
(emit `(= ,lhs ,rr))))
`(null))
(if rhs
(if (valid-ir-rvalue? lhs rhs)
(emit `(= ,lhs ,rhs))
(let ((rr (make-ssavalue)))
(emit `(= ,rr ,rhs))
(emit `(= ,lhs ,rr)))))
#f)
;; the interpreter loop. `break-labels` keeps track of the labels to jump to
;; for all currently closing break-blocks.
;; `value` means we are in a context where a value is required; a meaningful
Expand Down Expand Up @@ -3621,7 +3627,7 @@ f(x) = yt(x)
lhs)))
(if (and (not *warn-all-loop-vars*) (has? deprecated-loop-vars lhs))
(del! deprecated-loop-vars lhs))
(if value
(if (and value rhs)
(let ((rr (if (or (atom? rhs) (ssavalue? rhs) (eq? (car rhs) 'null))
rhs (make-ssavalue))))
(if (not (eq? rr rhs))
Expand Down Expand Up @@ -3671,7 +3677,7 @@ f(x) = yt(x)
(if file-diff (set! filename last-fname)))))
((return)
(compile (cadr e) break-labels #t #t)
'(null))
#f)
((unnecessary)
;; `unnecessary` marks expressions generated by lowering that
;; do not need to be evaluated if their value is unused.
Expand Down Expand Up @@ -3743,7 +3749,7 @@ f(x) = yt(x)
(emit `(goto ,m))
(set! handler-goto-fixups
(cons (list code handler-level (cadr e)) handler-goto-fixups))
'(null)))
#f))

;; exception handlers are lowered using
;; (enter L) - push handler with catch block at label L
Expand All @@ -3763,9 +3769,9 @@ f(x) = yt(x)
(let* ((v1 (compile (cadr e) break-labels value #f))
(val (if (and value (not tail))
(new-mutable-var) #f)))
(if val (emit-assignment val v1))
(if (and val v1) (emit-assignment val v1))
(if tail
(begin (emit-return v1)
(begin (if v1 (emit-return v1))
(if (not finally) (set! endl #f)))
(begin (emit '(leave 1))
(emit `(goto ,endl))))
Expand All @@ -3774,7 +3780,7 @@ f(x) = yt(x)
(emit `(leave 1))
(if finally
(begin (emit `(= ,finally-exception (the_exception)))
(leave-finally-block `(foreigncall 'jl_rethrow_other (core Nothing) (call (core svec) Any)
(leave-finally-block `(foreigncall 'jl_rethrow_other (top Bottom) (call (core svec) Any)
'ccall 1 ,finally-exception)
#f))
(let ((v2 (compile (caddr e) break-labels value tail)))
Expand Down
9 changes: 9 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,15 @@ function f24331()
end
@test f24331() == [2]

# issue #26743
function f26743()
try
return 5
finally
end
end
@test @inferred(f26743()) == 5

# finalizers
let A = [1]
local x = 0
Expand Down

0 comments on commit 43876e3

Please sign in to comment.