Skip to content

fix #27710, scope error with static param. and variable name conflicts #27760

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 25, 2018
Merged

fix #27710, scope error with static param. and variable name conflicts #27760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 36 additions & 37 deletions src/julia-syntax.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,15 +273,6 @@
(map (lambda (x) (replace-vars x renames))
(cdr e))))))

(define (replace-outer-vars e renames)
(cond ((and (pair? e) (eq? (car e) 'outerref)) (lookup (cadr e) renames e))
((or (not (pair? e)) (quoted? e)) e)
((memq (car e) '(-> function scope-block)) e)
(else
(cons (car e)
(map (lambda (x) (replace-outer-vars x renames))
(cdr e))))))

(define (make-generator-function name sp-names arg-names body)
(let ((arg-names (append sp-names
(map (lambda (n)
Expand Down Expand Up @@ -2508,7 +2499,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 outerglobals implicitglobals lam renames newlam)
(define (resolve-scopes- e env outerglobals implicitglobals lam renames newlam (sp '()))
(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 symbolicgoto symboliclabel))) e)
Expand All @@ -2525,7 +2516,7 @@
`(warn-loop-var ,(cadr e))
'(null)))
((eq? (car e) 'lambda)
(let* ((lv (lam:vars e))
(let* ((lv (append sp (lam:vars e)))
(env (append lv env))
(body (resolve-scopes- (lam:body e) env
;; don't propagate implicit or outer globals
Expand Down Expand Up @@ -2622,9 +2613,15 @@
`(break-block ,(cadr e) ;; ignore type symbol 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 outerglobals implicitglobals lam renames #f)
,@(cddr e))) ;; body of break-block expression
`(with-static-parameters
,(resolve-scopes- (cadr e) env outerglobals implicitglobals lam renames #f (cddr e))
,@(cddr e)))
((and (eq? (car e) 'method) (length> e 2))
`(method
,(resolve-scopes- (cadr e) env outerglobals implicitglobals lam renames #f)
,(resolve-scopes- (caddr e) env outerglobals implicitglobals lam renames #f)
,(resolve-scopes- (cadddr e) env outerglobals implicitglobals lam renames #f
(method-expr-static-parameters e))))
(else
(cons (car e)
(map (lambda (x)
Expand Down Expand Up @@ -3192,13 +3189,12 @@ f(x) = yt(x)
,@sp-inits
(method ,name ,(cl-convert sig fname lam namemap toplevel interp)
,(let ((body (add-box-inits-to-body
lam2
(cl-convert (cadddr lam2) 'anon lam2 (table) #f interp))))
lam2
(cl-convert (cadddr lam2) 'anon lam2 (table) #f interp))))
`(lambda ,(cadr lam2)
(,(clear-capture-bits (car vis))
,@(cdr vis))
,body))
,(last e))))
,body)))))
(else
(let* ((exprs (lift-toplevel (convert-lambda lam2 '|#anon| #t '())))
(top-stmts (cdr exprs))
Expand Down Expand Up @@ -3236,20 +3232,18 @@ f(x) = yt(x)
(capt-vars (diff all-capt-vars capt-sp)) ; remove capt-sp from capt-vars
(find-locals-in-method-sig (lambda (methdef)
(expr-find-all
(lambda (e) (and (or (symbol? e) (and (pair? e) (eq? (car e) 'outerref)))
(let ((s (if (symbol? e) e (cadr e))))
(and (symbol? s)
(not (eq? name s))
(not (memq s capt-sp))
(if (and (local? s) (length> (lam:args lam) 0))
; error for local variables except in toplevel thunks
(error (string "local variable " s
" cannot be used in closure declaration"))
#t)
; allow captured variables
(memq s (lam:sp lam))))))
(lambda (s) (and (symbol? s)
(not (eq? name s))
(not (memq s capt-sp))
(if (and (local? s) (length> (lam:args lam) 0))
;; error for local variables except in toplevel thunks
(error (string "local variable " s
" cannot be used in closure declaration"))
#t)
;; allow captured variables
(memq s (lam:sp lam))))
(caddr methdef)
(lambda (e) (cadr e)))))
identity)))
(sig-locals (simple-sort
(delete-duplicates ;; locals used in sig from all definitions
(apply append ;; will convert these into sparams for dispatch
Expand All @@ -3274,7 +3268,7 @@ f(x) = yt(x)
(let* ((iskw ;; TODO jb/functions need more robust version of this
(contains (lambda (x) (eq? x 'kwftype)) sig))
(renamemap (map cons closure-param-names closure-param-syms))
(arg-defs (replace-outer-vars
(arg-defs (replace-vars
(fix-function-arg-type sig type-name iskw namemap closure-param-syms)
renamemap)))
(append (map (lambda (gs tvar)
Expand All @@ -3296,7 +3290,7 @@ f(x) = yt(x)
v)))
capt-vars))
(P (append
(map (lambda (n) `(outerref ,n)) closure-param-names)
closure-param-names
(filter identity (map (lambda (v ve)
(if (is-var-boxed? v lam)
#f
Expand Down Expand Up @@ -4100,14 +4094,19 @@ f(x) = yt(x)
(define (renumber-stuff e)
(cond ((symbol? e)
(let ((idx (get slot-table e #f)))
(if idx `(slot ,idx) e)))
(if idx
`(slot ,idx)
(let ((idx (get sp-table e #f)))
(if idx
`(static_parameter ,idx)
e)))))
((and (pair? e) (eq? (car e) 'outerref))
(let ((idx (get sp-table (cadr e) #f)))
(if idx `(static_parameter ,idx) (cadr e))))
(cadr e))
((nospecialize-meta? e)
;; convert nospecialize vars to slot numbers
`(meta nospecialize ,@(map renumber-stuff (cddr e))))
((or (atom? e) (quoted? e)) e)
((or (atom? e) (quoted? e) (eq? (car e) 'global))
e)
((ssavalue? e)
(let ((idx (or (get ssavalue-table (cadr e) #f)
(error "ssavalue with no def"))))
Expand Down
8 changes: 8 additions & 0 deletions test/syntax.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1447,3 +1447,11 @@ f27129(x = 1) = (@Base._inline_meta; x)
for meth in methods(f27129)
@test ccall(:jl_uncompress_ast, Any, (Any, Any), meth, meth.source).inlineable
end

# issue #27710
struct Foo27710{T} end
function test27710()
types(::Foo27710{T}) where T = T
T = types(Foo27710{Int64}())
Copy link
Member

@StefanKarpinski StefanKarpinski Jun 25, 2018

Choose a reason for hiding this comment

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

This is a pretty inscrutable test. Is assigning this particular value to T significant or is it just the fact that something is being assigned to it that's important?

Copy link
Member Author

@JeffBezanson JeffBezanson Jun 25, 2018

Choose a reason for hiding this comment

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

Both. The inner function needs to work, and the assigned variable here needs to be called T.

Copy link
Member

@StefanKarpinski StefanKarpinski Jun 25, 2018

Choose a reason for hiding this comment

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

Right, but you could do something like this instead, right:

T = nothing
types(::Foo27710{T}) where T = T
return types(Foo27710{Int64}())

Copy link
Member Author

@JeffBezanson JeffBezanson Jun 25, 2018

Choose a reason for hiding this comment

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

Sure, or even better return (T, types(Foo27710{Int64}())), to make sure the assignment works too. But it seems like a pretty minor point to me.

Copy link
Member

@StefanKarpinski StefanKarpinski Jun 25, 2018

Choose a reason for hiding this comment

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

I was just trying to understand it, not complain 😄

end
@test test27710() === Int64