Function keyword arguments argument evaluation order

[yuyichao]

I was trying to figure out what is the order julia evaluate it's arguments (especially when there's keyword arguments passed in different forms). IIRC, python grantee that the arguments are evaluated in the order they appear in the code. Is there similar garentee in julia? I'm sorry if it is already in the document but at least I didn't find anything when I look through the page about functions.

I was trying to figure out the current order myself with some code and I see the following very unexpected result.

julia> function get_next()
           global counter
           counter = counter + 1
           return counter
       end
get_next (generic function with 1 method)

julia> 

julia> function get_type(typ::Type)
           id = get_next()
           println("$id: $typ")
           return typ
       end
get_type (generic function with 1 method)

julia> 

julia> function f(args...; kws...)
           println((args, kws))
       end
f (generic function with 1 method)

julia> 

julia> counter = 1
1

julia> f(get_next(), a=get_next(), get_next()::get_type(Int),
         b=get_next(), get_next()::get_type(Number),
         [get_next(), get_next()]...; c=get_next(),
         [(:d, get_next()), (:f, get_next())]...)
9: Int64
11: Number
((13,13,13,13,13),Any[(:a,13),(:b,13),(:c,13),(:d,3),(:f,3)])

julia> 

julia> counter = 1
1

julia> f(get_next(),; c=get_next(), [(:d, get_next()), (:f, get_next())]...)
((5,),Any[(:c,5),(:d,3),(:f,3)])

julia> counter = 1
1                                                                                  

julia> f(get_next(), a=get_next(), get_next()::get_type(Int),
         b=get_next(), get_next()::get_type(Number),
         [get_next(), get_next()]...; c=get_next())
7: Int64                                                                           
9: Number                                                                          
((5,6,8,10,11),Any[(:a,2),(:b,3),(:c,4)])                                          

julia> counter = 1
1

julia> kws = [(:d, get_next()), (:f, get_next())]
2-element Array{(Symbol,Int64),1}:
 (:d,2)
 (:f,3)

julia> f(get_next(), a=get_next(), get_next()::get_type(Int),
         b=get_next(), get_next()::get_type(Number),
         [get_next(), get_next()]...; c=get_next(),
         kws...)
9: Int64
11: Number
((13,13,13,13,13),Any[(:a,13),(:b,13),(:c,13),(:d,2),(:f,3)])

As shown above in the REPL output, the arguments passed to the function are fine as long as I'm not passing (/forwarding) keyword arguments with the (;kw...) syntax (is there a name for it?) and the arguments are evaluated in the order they appear in the code with the keyword arguments first and then the positional ones. However, whenever I'm trying to forward some keyword arguments, the whole thing gets messed up and the get_next function "appears to" return same value for multiple calls.

This issue disappears when I tweak the get_next function

julia> function get_next()
           global counter
           counter = counter + 1
           res = counter
           return res
       end
get_next (generic function with 1 method)

julia> counter = 1
1

julia> f(get_next(),; c=get_next(), [(:d, get_next()), (:f, get_next())]...)
((5,),Any[(:c,4),(:d,2),(:f,3)])

julia> counter = 1
1

julia> f(get_next(),; c=get_next(), [(:d, get_next()), (:f, get_next())]..., [(:e, get_next()), (:g, get_next())]...)
((7,),Any[(:c,6),(:d,2),(:f,3),(:e,4),(:g,5)])

It seems like sth related to the compiler/optimizer to me. It also shows that the forwarded kw args are evaluated before everything else (which is fine as long as it's consistent but a little bit weird....)

[yuyichao]

I'm surprised that having ,; in function argument is not an error but hopefully it's not related to this issue....

[yuyichao]

And forgot to mention, this is on latest master as of an hour ago.

Julia Version 0.4.0-dev+2352
Commit 29bfd0e* (2015-01-01 08:32 UTC)

[vtjnash]

it looks like there may be multiple bugs here:

1. the parsing pass for kw args moves them all to the front (as you observed), but may need to introduce temporary variables for the other arguments to keep them in the syntatic order
2. the effect_free computation is inlining the get_next function incorrectly, and perhaps not taking into account that counter is a global variable

allowing both ,; is necessary for writing f(a...; b...) and having a be positional and b be kw args

[yuyichao]

AFAIK, although the parsing pass does move all parameters after ; to the front in the AST, it is still reversable, i.e.

julia> dump(:(f(a, b, c=0, d, args...; e=0, kw...)), 100, "")
Expr 
  head: Symbol call
  args: Array(Any,(7,))
    1: Symbol f
    2: Expr 
      head: Symbol parameters
      args: Array(Any,(2,))
        1: Expr 
          head: Symbol kw
          args: Array(Any,(2,))
            1: Symbol e
            2: Int64 0
          typ: Any
        2: Expr 
          head: Symbol ...
          args: Array(Any,(1,))
            1: Symbol kw
          typ: Any
      typ: Any
    3: Symbol a
    4: Symbol b
    5: Expr 
      head: Symbol kw
      args: Array(Any,(2,))
        1: Symbol c
        2: Int64 0
      typ: Any
    6: Symbol d
    7: Expr 
      head: Symbol ...
      args: Array(Any,(1,))
        1: Symbol args
      typ: Any
  typ: Any

It will be the right order as long as the :parameters expression is evaluated at the end. Given that the interpreter/codegen already doesn't evaluate the arguments in the order in the AST after parsing (it move all :kw to the first) it should be possible to fix it without changing the parser?

I personally don't really mind to keep the current order either, but sticking to the syntatic order would be better.

The ,; thing is just an observation not a bug report...

Also, I just remembered another example of confusing printing of AST, related to #9503 and #9474.

julia> Expr(:(=), :c, 0)
:(c = 0)

julia> Expr(:kw, :c, 0)
:(c=0)

I guess this time they are technically printed differently and I'm also not sure how reasonably it is to parse A=B to the same expression this time...

[yuyichao]

Not sure if this is useful info but the bug is probably in type inference

julia> function get_next()
           global counter
           counter = counter + 1
           return counter
       end
get_next (generic function with 1 method)

julia> function g()
       global counter = 0
       f(;[(:d, get_next()), (:f, get_next())]...,)
       end
g (generic function with 1 method)

julia> function f(args...; kws...)
           println((args, kws))
       end
f (generic function with 1 method)

julia> g()
((),Any[(:d,2),(:f,2)])

julia> code_lowered(g, ())
1-element Array{Any,1}:
 :($(Expr(:lambda, Any[], Any[Any[symbol("#s452"),symbol("#s449"),symbol("#s448"),symbol("#s447"),symbol("#s446"),symbol("#s444"),symbol("#s451"),symbol("#s443"),symbol("#s450"),symbol("#s445")],Any[Any[symbol("#s452"),:Any,18],Any[symbol("#s449"),:Any,18],Any[symbol("#s448"),:Any,2],Any[symbol("#s447"),:Any,18],Any[symbol("#s446"),:Any,18],Any[symbol("#s444"),:Any,18],Any[symbol("#s451"),:Any,18],Any[symbol("#s443"),:Any,18],Any[symbol("#s450"),:Any,18],Any[symbol("#s445"),:Any,2]],Any[]], :(begin  # none, line 2:
        counter = 0 # line 3:
        #s452 = (top(Array))(top(Any),0)::Any
        #s449 = vcat((top(tuple))(:d,get_next()::Any)::Any,(top(tuple))(:f,get_next()::Any)::Any)::Any
        #s448 = (top(start))(#s449)::Any
        unless (top(!))((top(done))(#s449,#s448)::Any)::Any goto 1
        2: 
        #s447 = (top(next))(#s449,#s448)::Any
        #s446 = (top(tupleref))(#s447,1)::Any
        #s445 = (top(start))(#s446)::Any
        #s444 = ((top(getfield))(top(Base),:indexed_next)::Any)(#s446,1,#s445)::Any
        #s451 = (top(tupleref))(#s444,1)::Any
        #s445 = (top(tupleref))(#s444,2)::Any
        #s443 = ((top(getfield))(top(Base),:indexed_next)::Any)(#s446,2,#s445)::Any
        #s450 = (top(tupleref))(#s443,1)::Any
        #s445 = (top(tupleref))(#s443,2)::Any
        #s448 = (top(tupleref))(#s447,2)::Any
        (top(ccall))(:jl_cell_1d_push2,Void,(top(tuple))(Any,Any,Any)::Any,#s452,0,(top(typeassert))(#s451,top(Symbol))::Any,0,#s450,0)::Any
        3: 
        unless (top(!))((top(!))((top(done))(#s449,#s448)::Any)::Any)::Any goto 2
        1: 
        0: 
        unless (top(isempty))(#s452)::Any goto 4
        return f()::Any
        4: 
        return (top(kwcall))(call,0,f,#s452)::Any
    end::Any))))

julia> code_typed(g, ())
1-element Array{Any,1}:
 :($(Expr(:lambda, Any[], Any[Any[symbol("#s452"),symbol("#s449"),symbol("#s448"),symbol("#s447"),symbol("#s446"),symbol("#s444"),symbol("#s451"),symbol("#s443"),symbol("#s450"),symbol("#s445"),:_var1,:_var0,:_var2,:_var3],Any[Any[symbol("#s452"),Array{Any,1},18],Any[symbol("#s449"),Any,18],Any[symbol("#s448"),Any,2],Any[symbol("#s447"),Any,18],Any[symbol("#s446"),Any,18],Any[symbol("#s444"),Any,18],Any[symbol("#s451"),Any,18],Any[symbol("#s443"),Any,18],Any[symbol("#s450"),Any,18],Any[symbol("#s445"),Any,2],Any[:_var1,Int64,18],Any[:_var0,Array{Any,1},18],Any[:_var2,Array{Any,1},18],Any[:_var3,(((),Array{Any,1}),),0]],Any[]], :(begin  # none, line 2:
        counter = 0 # line 3:
        #s452 = (top(ccall))(:jl_alloc_array_1d,$(Expr(:call1, :(top(apply_type)), :Array, Any, 1)),$(Expr(:call1, :(top(tuple)), :Any, :Int)),Array{Any,1},0,0,0)::Array{Any,1}
        counter = counter + 1::Any
        counter = counter + 1::Any
        #s449 = vcat((top(tuple))(:d,counter)::(Symbol,Any),(top(tuple))(:f,counter)::(Symbol,Any))::Any
        #s448 = (top(start))(#s449)::Any
        unless (top(!))((top(done))(#s449,#s448)::Any)::Any goto 1
        2: 
        #s447 = (top(next))(#s449,#s448)::Any
        #s446 = (top(tupleref))(#s447,1)::Any
        #s445 = (top(start))(#s446)::Any
        #s444 = ((top(getfield))(top(Base),:indexed_next)::Any)(#s446,1,#s445)::Any
        #s451 = (top(tupleref))(#s444,1)::Any
        #s445 = (top(tupleref))(#s444,2)::Any
        #s443 = ((top(getfield))(top(Base),:indexed_next)::Any)(#s446,2,#s445)::Any
        #s450 = (top(tupleref))(#s443,1)::Any
        #s445 = (top(tupleref))(#s443,2)::Any
        #s448 = (top(tupleref))(#s447,2)::Any
        (top(ccall))(:jl_cell_1d_push2,Void,$(Expr(:call1, :(top(tuple)), :Any, :Any, :Any)),#s452::Array{Any,1},0,(top(typeassert))(#s451,top(Symbol))::Symbol,0,#s450,0)::Void
        3: 
        unless (top(!))((top(!))((top(done))(#s449,#s448)::Any)::Any)::Any goto 2
        1: 
        0: 
        _var1 = (top(arraylen))(#s452::Array{Any,1})::Int64
        unless _var1::Int64 === 0::Bool goto 4
        _var0 = (top(ccall))(:jl_alloc_array_1d,$(Expr(:call1, :(top(apply_type)), :Array, Any, 1)),$(Expr(:call1, :(top(tuple)), :Any, :Int)),Array{Any,1},0,0,0)::Array{Any,1}
        _var2 = _var0::Array{Any,1}
        return println(GetfieldNode(Base,:STDOUT,Any),(top(tuple))($(Expr(:call1, :(top(tuple)))),_var2::Array{Any,1})::((),Array{Any,1}))::Any
        4: 
        return (top(kwcall))(call,0,f,#s452::Array{Any,1})::Any
    end::Any))))

The lowered code looks fine but the typed code is clearly wrong....

[timholy]

This is well-known: http://docs.julialang.org/en/latest/manual/performance-tips/#avoid-global-variables. See the item about annotating them at point-of-use.

[yuyichao]

@timholy Which part are you refering to? I think this issue is more about correctness than performance.

[timholy]

Sorry, I thought you were worried about all the Any annotations.

[yuyichao]

The issue is with this part

counter = counter + 1::Any
counter = counter + 1::Any
#s449 = vcat((top(tuple))(:d,counter)::(Symbol,Any),(top(tuple))(:f,counter)::(Symbol,Any))::Any

The compiler decided to increment the global variable twice before the value is used. Type inference not being able to decide the type of counter is not the issue here

[tkelman]

should d071904 be scheduled for backporting?

[vtjnash]

Yes

[JeffBezanson]

It looks like maybe only the top half of the patch to inference.jl applies to 0.3?

[vtjnash]

i thought both parts would still be applicable. the second part ensures that we compute affect_free even if occ > 6 (previously, it was assuming that inline_worthy would return false for occ > 6 and so it skipped finishing the computation of affect_free. but that's not a good assumption anymore)

[JeffBezanson]

Ah, you're right, I thought 0.3 had less of the newer inlining logic.

[tkelman]

d071904 seems like another safer-for-0.3.6 change

[tkelman]

backported in 41bfd37

[mauro3]

I stumbled across this, which might be related. Certainly fits with the title:

julia> f(;a=5, b=a+c, c=6) = a+b+c
f (generic function with 1 method)

julia> f()
ERROR: UndefVarError: c not defined

julia> f(a=4)
20

I would expect both to fail as keywords are supposed to be evaluated left to right and c is not touched. (Let me know if I should open a separate issue instead)

[StefanKarpinski]

Positional argument defaults are evaluated in left-to-right order but keyword argument defaults are evaluated all-at-once, which leaves things kind of inconsistent, imo. I'd argue that keyword argument defaults should also be evaluated in left-to-right order.

[mauro3]

The manual says "the [keyword] default expressions are evaluated left-to-right" but all in the same scope.

[StefanKarpinski]

Right, the scoping is what I meant.

[stevengj]

A possibly related issue came up on julia-users:

julia> x = 3
3

julia> f(; x=x) = x
f (generic function with 1 method)

julia> f()
ERROR: UndefVarError: x not defined
 in f() at ./REPL[2]:1

[yuyichao]

No it's #17240

[tkelman]

has not seen any work for 0.6 timeframe

[JeffBezanson]

The issue with splatted keywords is fixed:

julia> counter = 1
1

julia> f(get_next(),; c=get_next(), [(:d, get_next()), (:f, get_next())]...)
((5,), Any[(:c, 2), (:d, 3), (:f, 4)])

julia> counter = 1
1

julia> f(get_next(),; c=get_next(), [(:d, get_next()), (:f, get_next())]..., [(:e, get_next()), (:g, get_next())]...)
((7,), Any[(:c, 2), (:d, 3), (:f, 4), (:e, 5), (:g, 6)])

The remaining issue is that keyword arguments are evaluated before positional arguments. That comes out weird in cases like

f(g(), a=g(), g())

I suppose we should make everything strictly left-to-right.

[yuyichao]

Yeah, the fix was when this issue was renamed with the bug and priority label removed. (edit: and apparently with the bug label added back again?....)