Skip to content

Commit

Permalink
always re-initialize variables to undefined at the top of their scope
Browse files Browse the repository at this point in the history
fixes #11065
  • Loading branch information
JeffBezanson committed May 29, 2015
1 parent 6b35206 commit ebfebfd
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 6 deletions.
39 changes: 33 additions & 6 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -3232,11 +3232,7 @@ So far only the second case can actually occur.
(vinf (var-info-for vname vi)))
(if (and vinf
(not (and (pair? code)
(equal? (car code) `(newvar ,vname))))
;; TODO: remove the following expression to re-null
;; all variables when they are allocated. see issue #1571
(vinfo:capt vinf)
)
(equal? (car code) `(newvar ,vname)))))
(emit `(newvar ,vname))
#f)))
((newvar)
Expand All @@ -3246,10 +3242,41 @@ So far only the second case can actually occur.
#f))
(else (emit (goto-form e))))))
(compile e '())
(cons 'body (reverse! code))))
(let* ((stmts (reverse! code))
(di (definitely-initialized-vars stmts vi)))
(cons 'body (filter (lambda (e)
(not (and (pair? e) (eq? (car e) 'newvar)
(has? di (cadr e)))))
stmts)))))

(define to-goto-form goto-form)

;; find newvar nodes that are unnecessary because (1) the variable is not
;; captured, and (2) the variable is assigned before any branches.
;; this is used to remove newvar nodes that are not needed for re-initializing
;; variables to undefined (see issue #11065). it doesn't look for variable
;; *uses*, because any variables used-before-def that also pass this test
;; are *always* used undefined, and therefore don't need to be *re*-initialized.
(define (definitely-initialized-vars stmts vi)
(let ((vars (table))
(di (table)))
(let loop ((stmts stmts))
(if (null? stmts)
di
(begin
(let ((e (car stmts)))
(cond ((and (pair? e) (eq? (car e) 'newvar))
(let ((vinf (var-info-for (cadr e) vi)))
(if (not (vinfo:capt vinf))
(put! vars (cadr e) #t))))
((and (pair? e) (eq? (car e) '=))
(if (has? vars (cadr e))
(begin (del! vars (cadr e))
(put! di (cadr e) #t))))
((and (pair? e) (memq (car e) '(goto gotoifnot)))
(set! vars (table)))))
(loop (cdr stmts)))))))

;; macro expander

(define (splice-expr? e)
Expand Down
12 changes: 12 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2913,3 +2913,15 @@ end
# issue #11366
f11366{T}(x::Type{Ref{T}}) = Ref{x}
@test !isleaftype(Base.return_types(f11366, (Any,))[1])

# issue #11065, #1571
function f11065()
for i = 1:2
if i == 1
z = "z is defined"
elseif i == 2
print(z)
end
end
end
@test_throws UndefVarError f11065()

6 comments on commit ebfebfd

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost certainly unrelated, but @carnaval any idea what would cause

julia-debug: gc.c:1071: __pool_alloc: Assertion `pg->osize == p->osize' failed.

here? https://travis-ci.org/JuliaLang/julia/jobs/64523844

@carnaval
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum, strange corruption or very worrying logic error in the gc. Is this the first time you see this one happening ?

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, but I haven't looked through every travis failure - might be worth doing some API-scraping and programmatically trying to categorize the different intermittent failures we've been getting

@amitmurthy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block of conditionally executed tests

julia/test/parallel.jl

Lines 211 to 244 in a031f93

if Bool(parse(Int,(get(ENV, "JULIA_TESTFULL", "0"))))
print("\n\nTesting correct error handling in pmap call. Please ignore printed errors when specified.\n")
# make sure exceptions propagate when waiting on Tasks
@test_throws ErrorException (@sync (@async error("oops")))
# pmap tests
# needs at least 4 processors (which are being created above for the @parallel tests)
s = "a"*"bcdefghijklmnopqrstuvwxyz"^100;
ups = "A"*"BCDEFGHIJKLMNOPQRSTUVWXYZ"^100;
@test ups == bytestring(UInt8[UInt8(c) for c in pmap(x->uppercase(x), s)])
@test ups == bytestring(UInt8[UInt8(c) for c in pmap(x->uppercase(Char(x)), s.data)])
# retry, on error exit
res = pmap(x->(x=='a') ? error("EXPECTED TEST ERROR. TO BE IGNORED.") : uppercase(x), s; err_retry=true, err_stop=true);
@test length(res) < length(ups)
@test isa(res[1], Exception)
# no retry, on error exit
res = pmap(x->(x=='a') ? error("EXPECTED TEST ERROR. TO BE IGNORED.") : uppercase(x), s; err_retry=false, err_stop=true);
@test length(res) < length(ups)
@test isa(res[1], Exception)
# retry, on error continue
res = pmap(x->iseven(myid()) ? error("EXPECTED TEST ERROR. TO BE IGNORED.") : uppercase(x), s; err_retry=true, err_stop=false);
@test length(res) == length(ups)
@test ups == bytestring(UInt8[UInt8(c) for c in res])
# no retry, on error continue
res = pmap(x->(x=='a') ? error("EXPECTED TEST ERROR. TO BE IGNORED.") : uppercase(x), s; err_retry=false, err_stop=false);
@test length(res) == length(ups)
@test isa(res[1], Exception)
print("\n\nPassed all pmap tests that print errors.\n")
has been failing sometimes on my local system . I wonder if pmap is affected by this commit.

@amitmurthy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I think I found the reason for the above.

@timholy
Copy link
Member

@timholy timholy commented on ebfebfd Jun 6, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this is the source of #11595 and #10980 (comment)

Please sign in to comment.