Skip to content

Commit

Permalink
fixing a long-standing type inference bug on variables assigned in
Browse files Browse the repository at this point in the history
  inner functions
  • Loading branch information
JeffBezanson committed Aug 31, 2011
1 parent efec328 commit 6c68fde
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 22 deletions.
7 changes: 7 additions & 0 deletions doc/todo
Original file line number Diff line number Diff line change
Expand Up @@ -797,3 +797,10 @@ this (and tintersect on NTuple) is needed to define Array() methods in boot.j

- check what function vcat{T}(A::Array{T,2}...) gets specialized for on
4 Array{Float64,2} arguments.

- make sure an argument with declared type gets the declaration recorded
even if the argument doesn't occur in the function (but might occur in
a nested function)

- call inference on general thunks so we don't need to make GFs for them
in toplevel_eval
23 changes: 18 additions & 5 deletions j/inference.j
Original file line number Diff line number Diff line change
Expand Up @@ -944,10 +944,23 @@ function typeinf(linfo::LambdaStaticData,atypes::Tuple,sparams::Tuple, cop, def)
end
# types of closed vars
cenv = idtable()
for vi = ast.args[2].args[3]
for vi = ((ast.args[2].args[3])::Array{Any,1})
vi::Array{Any,1}
cenv[vi[1]] = vi[2]
s[1][vi[1]] = vi[2]
vname = vi[1]
vtype = vi[2]
cenv[vname] = vtype
s[1][vname] = vtype
end
for vi = ((ast.args[2].args[2])::Array{Any,1})
vi::Array{Any,1}
if (vi[3]&4)!=0
# variables assigned by inner functions are treated like
# closed variables; we only use the declared type
vname = vi[1]
vtype = vi[2]
cenv[vname] = vtype
s[1][vname] = vtype
end
end
sv = StaticVarInfo(sparams, cenv)

Expand Down Expand Up @@ -1301,7 +1314,7 @@ function inlineable(f, e::Expr, vars)
end
end
for vi = meth[3].ast.args[2].args[2]
if vi[3]
if (vi[3]&1)!=0
# captures variables (TODO)
return NF
end
Expand Down Expand Up @@ -1435,7 +1448,7 @@ function inlining_pass(e::Expr, vars)
end
function add_variable(ast, name, typ)
vinf = {name,typ,false,true}
vinf = {name,typ,2}
locllist = (ast.args[2].args[1]::Expr).args
vinflist = ast.args[2].args[2]::Array{Any,1}
push(locllist, name)
Expand Down
1 change: 1 addition & 0 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,7 @@ static jl_value_t *copy_ast(jl_value_t *expr, jl_tuple_t *sp)
return expr;
}

// TODO: eval decl types for arguments of non-generic functions
static void eval_decl_types(jl_array_t *vi, jl_tuple_t *spenv)
{
size_t i;
Expand Down
2 changes: 1 addition & 1 deletion src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ jl_value_t *jl_toplevel_eval_flex(jl_value_t *ex, int fast)
jl_array_t *vinfos = jl_lam_vinfo((jl_expr_t*)thk->ast);
int i;
for(i=0; i < vinfos->length; i++) {
if (jl_cellref(jl_cellref(vinfos,i),2)!=jl_false) {
if (jl_vinfo_capt((jl_array_t*)jl_cellref(vinfos,i))) {
// interpreter doesn't handle closure environment
ewc = 1;
break;
Expand Down
6 changes: 3 additions & 3 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1481,8 +1481,8 @@ static void emit_function(jl_lambda_info_t *lam, Function *f)
jl_array_t *vi = (jl_array_t*)jl_cellref(vinfos, i);
assert(jl_is_array(vi));
char *vname = ((jl_sym_t*)jl_cellref(vi,0))->name;
isAssigned[vname] = (jl_cellref(vi,3)!=jl_false);
isCaptured[vname] = (jl_cellref(vi,2)!=jl_false);
isAssigned[vname] = (jl_vinfo_assigned(vi)!=0);
isCaptured[vname] = (jl_vinfo_capt(vi)!=0);
declTypes[vname] = jl_cellref(vi,1);
}
vinfos = jl_lam_capt(ast);
Expand All @@ -1491,7 +1491,7 @@ static void emit_function(jl_lambda_info_t *lam, Function *f)
assert(jl_is_array(vi));
char *vname = ((jl_sym_t*)jl_cellref(vi,0))->name;
closureEnv[vname] = i;
isAssigned[vname] = (jl_cellref(vi,3)!=jl_false);
isAssigned[vname] = (jl_vinfo_assigned(vi)!=0);
isCaptured[vname] = true;
declTypes[vname] = jl_cellref(vi,1);
}
Expand Down
43 changes: 30 additions & 13 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -1381,13 +1381,26 @@ So far only the second case can actually occur.
(flatten-scopes x)))
e))))

(define (make-var-info name) (list name 'Any #f #f))
(define (make-var-info name) (list name 'Any 0))
(define vinfo:name car)
(define vinfo:type cadr)
(define vinfo:capt caddr)
(define (vinfo:capt v) (< 0 (logand (caddr v) 1)))
(define (vinfo:set-type! v t) (set-car! (cdr v) t))
(define (vinfo:set-capt! v c) (set-car! (cddr v) c))
(define (vinfo:set-asgn! v a) (set-car! (cdddr v) a))
;; record whether var is captured
(define (vinfo:set-capt! v c) (set-car! (cddr v)
(if c
(logior (caddr v) 1)
(logand (caddr v) -2))))
;; whether var is assigned
(define (vinfo:set-asgn! v a) (set-car! (cddr v)
(if a
(logior (caddr v) 2)
(logand (caddr v) -3))))
;; whether var is assigned by an inner function
(define (vinfo:set-iasg! v a) (set-car! (cddr v)
(if a
(logior (caddr v) 4)
(logand (caddr v) -5))))
(define var-info-for assq)

(define (lambda-all-vars e)
Expand All @@ -1405,16 +1418,19 @@ So far only the second case can actually occur.
; convert each lambda's (locals ...) to
; (vinf (locals ...) var-info-lst captured-var-infos)
; where var-info-lst is a list of var-info records
(define (analyze-vars e env)
(define (analyze-vars e env captvars)
(cond ((or (atom? e) (quoted? e)) e)
((and (eq? (car e) '=) (symbol? (cadr e)))
((eq? (car e) '=)
(let ((vi (var-info-for (cadr e) env)))
(if vi
(vinfo:set-asgn! vi #t)))
`(= ,(cadr e) ,(analyze-vars (caddr e) env)))
(begin
(vinfo:set-asgn! vi #t)
(if (assq (car vi) captvars)
(vinfo:set-iasg! vi #t)))))
`(= ,(cadr e) ,(analyze-vars (caddr e) env captvars)))
((or (eq? (car e) 'local) (eq? (car e) 'local!))
(if (pair? (cadr e))
(analyze-vars (cadr e) env)
(analyze-vars (cadr e) env captvars)
'(null)))
((eq? (car e) 'typeassert)
;(let ((vi (var-info-for (cadr e) env)))
Expand Down Expand Up @@ -1455,7 +1471,8 @@ So far only the second case can actually occur.
(and
(not (memq (vinfo:name v) allv))
(not (memq (vinfo:name v) glo))))
env)))))
env))
cv)))
; mark all the vars we capture as captured
(for-each (lambda (v) (vinfo:set-capt! v #t))
cv)
Expand All @@ -1474,12 +1491,12 @@ So far only the second case can actually occur.
(analyze-vars
`(call (lambda ,vs ,(caddr (cadr e)) ,(cadddr (cadr e)))
,@vs)
env))))
env captvars))))
(else (cons (car e)
(map (lambda (x) (analyze-vars x env))
(map (lambda (x) (analyze-vars x env captvars))
(cdr e))))))

(define (analyze-variables e) (analyze-vars e '()))
(define (analyze-variables e) (analyze-vars e '() '()))

; remove if, _while, block, break-block, and break
; replaced with goto and gotoifnot
Expand Down
15 changes: 15 additions & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,21 @@ jl_expr_t *jl_lam_body(jl_expr_t *l);
jl_sym_t *jl_decl_var(jl_value_t *ex);
DLLEXPORT int jl_is_rest_arg(jl_value_t *ex);

static inline int jl_vinfo_capt(jl_array_t *vi)
{
return (jl_unbox_long(jl_cellref(vi,2))&1)!=0;
}

static inline int jl_vinfo_assigned(jl_array_t *vi)
{
return (jl_unbox_long(jl_cellref(vi,2))&2)!=0;
}

static inline int jl_vinfo_assigned_inner(jl_array_t *vi)
{
return (jl_unbox_long(jl_cellref(vi,2))&4)!=0;
}

// for writing julia functions in C
#define JL_CALLABLE(name) \
jl_value_t *name(jl_value_t *env, jl_value_t **args, uint32_t nargs)
Expand Down

0 comments on commit 6c68fde

Please sign in to comment.