Skip to content

Commit

Permalink
fix scope rules: implicitly using a global doesn't add it to the scope
Browse files Browse the repository at this point in the history
  • Loading branch information
vtjnash committed May 25, 2017
1 parent 62e8227 commit 3900e33
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 50 deletions.
1 change: 1 addition & 0 deletions base/REPL.jl
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,7 @@ function respond(f, repl, main; pass_empty = false)
line = String(take!(buf))
if !isempty(line) || pass_empty
reset(repl)
local val, bt
try
# note: value wrapped carefully here to ensure it doesn't get passed through expand
response = eval(Main, Expr(:body, Expr(:return, Expr(:call, QuoteNode(f), QuoteNode(line)))))
Expand Down
2 changes: 1 addition & 1 deletion base/libgit2/libgit2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ function isdiff(repo::GitRepo, treeish::AbstractString, paths::AbstractString=""
diff = diff_tree(repo, tree, paths, cached=cached)
result = count(diff) > 0
close(diff)
return result
finally
close(tree)
end
return result
end

"""
Expand Down
29 changes: 14 additions & 15 deletions src/jlfrontend.scm
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
(cdr e))))
tab)))

;; find variables that should be forced to be global in a toplevel expr
(define (find-possible-globals e)
(table.keys (find-possible-globals- e (table))))

Expand All @@ -59,19 +60,6 @@
(define (some-gensym? x)
(or (gensym? x) (memq x *gensyms*)))

;; find variables that should be forced to be global in a toplevel expr
(define (toplevel-expr-globals e)
(diff
(delete-duplicates
(append
;; vars assigned at the outer level
(filter (lambda (x) (not (some-gensym? x))) (find-assigned-vars e '()))
;; vars declared const or global outside any scope block
(find-decls 'const e)
(find-decls 'global e)
;; vars assigned anywhere, if they have been defined as global
(filter defined-julia-global (find-possible-globals e))))
(find-decls 'local e)))

;; return a lambda expression representing a thunk for a top-level expression
;; note: expansion of stuff inside module is delayed, so the contents obey
Expand All @@ -81,11 +69,22 @@
(if (and (pair? ex0) (eq? (car ex0) 'toplevel))
ex0
(let* ((ex (julia-expand0 ex0))
(gv (toplevel-expr-globals ex))
(lv (find-decls 'local ex))
(gv (diff (delete-duplicates
(append (find-decls 'const ex) ;; convert vars declared const outside any scope block to outer-globals
(find-decls 'global ex) ;; convert vars declared global outside any scope block to outer-globals
;; vars assigned at the outer level
(filter (lambda (x) (not (some-gensym? x)))
(find-assigned-vars ex '()))))
lv))
;; vars assigned anywhere, if they have not been explicitly defined
(existing-gv (filter (lambda (x) (and (not (or (memq x lv) (memq x gv))) (defined-julia-global x)))
(find-possible-globals ex)))
(th (julia-expand1
`(lambda () ()
(scope-block
(block ,@(map (lambda (v) `(implicit-global ,v)) gv)
(block ,@(map (lambda (v) `(implicit-global ,v)) existing-gv)
,@(map (lambda (v) `(implicit-global ,v)) gv)
,ex))))))
(if (and (null? (cdadr (caddr th)))
(= 0 (cadddr (caddr th))))
Expand Down
54 changes: 25 additions & 29 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -2500,7 +2500,7 @@
;; 3. variables assigned inside this scope-block that don't exist in outer
;; scopes
;; returns lambdas in the form (lambda (args...) (locals...) body)
(define (resolve-scopes- e env implicitglobals lam renames newlam)
(define (resolve-scopes- e env outerglobals implicitglobals lam renames newlam)
(cond ((symbol? e) (let ((r (assq e renames)))
(if r (cdr r) e))) ;; return the renaming for e, or e
((or (not (pair? e)) (quoted? e) (memq (car e) '(toplevel global))) e)
Expand All @@ -2511,28 +2511,28 @@
(let* ((lv (lam:vars e))
(env (append lv env))
(body (resolve-scopes- (lam:body e) env
;; don't propagate implicit globals
;; issue #7234
;; don't propagate implicit or outer globals
'()
'()
e
;; remove renames corresponding to local variables from the environment
(filter (lambda (ren) (not (memq (car ren) lv)))
renames)
#t)))
`(lambda ,(cadr e) ,(caddr e) ,body)))
((eq? (car e) 'scope-block)
(let* ((blok (cadr e)) ;; body of scope-block expression
(other-locals (if lam (caddr lam) '())) ;; locals that are explicitly part of containing lambda expression
(iglo (find-decls 'implicit-global blok)) ;; implicitly defined globals used in blok
(iglo (find-decls 'implicit-global blok)) ;; globals defined implicitly outside blok
(glob (diff (find-global-decls blok) iglo)) ;; all globals declared in blok
(vars-def (check-dups (find-local-def-decls blok) '()))
(locals-declared (check-dups (find-local-decls blok) vars-def))
(locals-implicit (diff (implicit-locals
blok
;; being declared global prevents a variable
;; assignment from introducing a local
(append env glob implicitglobals iglo)
(append glob iglo))
vars-def))
(locals-implicit (implicit-locals
blok
;; being declared global prevents a variable
;; assignment from introducing a local
(append env glob iglo outerglobals locals-declared vars-def)
(append glob iglo)))
(vars (delete-duplicates (append! locals-declared locals-implicit)))
(all-vars (append vars vars-def))
(need-rename?
Expand All @@ -2551,26 +2551,22 @@
(renamed (map named-gensy need-rename))
(renamed-def (map named-gensy need-rename-def))
(new-env (append all-vars glob env)) ;; all variables declared in or outside blok
(new-iglo-table ;; initial list of implicit globals from outside blok which aren't part of the local vars
(let ((tab (table)))
(for-each (lambda (v) (if (not (memq v all-vars)) (put! tab v #t))) iglo)
(for-each (lambda (v) (if (not (memq v all-vars)) (put! tab v #t))) implicitglobals)
tab))
(new-iglo (table.keys ;; compute list of all globals used implicitly in blok
(unbound-vars blok
new-env ;; list of everything else
new-iglo-table)))
;; compute list of all globals used implicitly in blok (need renames)
(new-iglo (table.keys (unbound-vars blok
new-env ;; list of everything else
(table))))
;; combine the list of new renamings with the inherited list
(new-renames (append (map cons need-rename renamed) ;; map from definition name -> gensym name
(map cons need-rename-def renamed-def)
(map (lambda (g) (cons g `(outerref ,g))) new-iglo)
(filter (lambda (ren) ;; old renames list, with anything in vars removed
(not (or (memq (car ren) all-vars)
(memq (car ren) iglo)
(memq (car ren) implicitglobals)
(memq (car ren) glob))))
(let ((var (car ren)))
(not (or (memq var all-vars) ;; remove anything new
(memq var implicitglobals) ;; remove anything only added implicitly in the last scope block
(memq var glob))))) ;; remove anything that's now global
renames)))
(body (resolve-scopes- blok new-env new-iglo lam new-renames #f))
(new-oglo (append iglo outerglobals)) ;; list of all outer-globals from outside blok
(body (resolve-scopes- blok new-env new-oglo new-iglo lam new-renames #f))
(real-new-vars (append (diff vars need-rename) renamed))
(real-new-vars-def (append (diff vars-def need-rename-def) renamed-def)))
(for-each (lambda (v)
Expand All @@ -2590,18 +2586,18 @@
(error "module expression not at top level"))
((eq? (car e) 'break-block)
`(break-block ,(cadr e) ;; ignore type symbol of break-block expression
,(resolve-scopes- (caddr e) env implicitglobals lam renames #f))) ;; body of break-block expression
,(resolve-scopes- (caddr e) env outerglobals implicitglobals lam renames #f))) ;; body of break-block expression
((eq? (car e) 'with-static-parameters)
`(with-static-parameters ;; ignore list of sparams in break-block expression
,(resolve-scopes- (cadr e) env implicitglobals lam renames #f)
,(resolve-scopes- (cadr e) env outerglobals implicitglobals lam renames #f)
,@(cddr e))) ;; body of break-block expression
(else
(cons (car e)
(map (lambda (x)
(resolve-scopes- x env implicitglobals lam renames #f))
(resolve-scopes- x env outerglobals implicitglobals lam renames #f))
(cdr e))))))

(define (resolve-scopes e) (resolve-scopes- e '() '() #f '() #f))
(define (resolve-scopes e) (resolve-scopes- e '() '() '() #f '() #f))

;; pass 3: analyze variables

Expand Down
78 changes: 73 additions & 5 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -438,16 +438,27 @@ glotest()
@test loc_x == 10

# issue #7234
f7234_cnt = 0
begin
glob_x2 = 24
f7234_a() = (glob_x2 += 1)
function f7234_a()
global f7234_cnt += 1
glob_x2 += 1
global f7234_cnt += -10000
end
end
@test_throws UndefVarError f7234_a()
@test f7234_cnt == 1
begin
global glob_x2 = 24
f7234_b() = (glob_x2 += 1)
function f7234_b()
global f7234_cnt += 1
glob_x2 += 1
global f7235_cnt += -10000
end
end
@test_throws UndefVarError f7234_b()
@test f7234_cnt == 2
# existing globals can be inherited by non-function blocks
for i = 1:2
glob_x2 += 1
Expand Down Expand Up @@ -486,16 +497,23 @@ end
@test h19333() == 4

# let - new variables, including undefinedness
let_undef_cnt = 0
function let_undef()
first = true
for i = 1:2
let x
if first; x=1; first=false; end
x+1
let x # new x
if first # not defined on second pass
x = 1
first = false
end
global let_undef_cnt += 1
x + 1
global let_undef_cnt += 23
end
end
end
@test_throws UndefVarError let_undef()
@test let_undef_cnt == 25

# const implies local in a local scope block
function const_implies_local()
Expand All @@ -521,6 +539,56 @@ end
@test a[2](10) == 12
@test a[3](10) == 13

# issue #21900
f21900_cnt = 0
function f21900()
for i = 1:1
x = 0
end
global f21900_cnt += 1
x
global f21900_cnt += -1000
nothing
end
@test_throws UndefVarError f21900()
@test f21900_cnt == 1

@test_throws UndefVarError @eval begin
for i21900 = 1:10
for j21900 = 1:10
foo21900 = 10
end
bar21900 = 0
bar21900 = foo21900 + 1
end
end
@test !isdefined(:foo21900)
@test !isdefined(:bar21900)
bar21900 = 0
@test_throws UndefVarError @eval begin
for i21900 = 1:10
for j21900 = 1:10
foo21900 = 10
end
bar21900 = -1
bar21900 = foo21900 + 1
end
end
@test bar21900 == -1
@test !isdefined(:foo21900)
foo21900 = 0
@test nothing === @eval begin
for i21900 = 1:10
for j21900 = 1:10
foo21900 = 10
end
bar21900 = -1
bar21900 = foo21900 + 1
end
end
@test foo21900 == 10
@test bar21900 == 11

# ? syntax
@test (true ? 1 : false ? 2 : 3) == 1

Expand Down
1 change: 1 addition & 0 deletions test/libgit2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1628,6 +1628,7 @@ mktempdir() do dir

loopback = ip"127.0.0.1"
for hostname in hostnames
local addr
try
addr = getaddrinfo(hostname)
catch
Expand Down

0 comments on commit 3900e33

Please sign in to comment.