jw3126 / Setfield.jl

Update deeply nested immutable structs.
Other
167 stars 17 forks source link

Should `@set` mutate things? #22

Closed jw3126 closed 6 years ago

jw3126 commented 6 years ago

Currently @set will mutate things if possible:

julia> using Setfield
INFO: Recompiling stale cache file /home/jan/.julia/lib/v0.6/Setfield.ji for module Setfield.

julia> v = [0]
1-element Array{Int64,1}:
 0

julia> @set v[1] = 10
1-element Array{Int64,1}:
 10

julia> v
1-element Array{Int64,1}:
 10

is this a good idea? It currently also means that @set can not change the type in the mutable situation

julia> v[1] = 1.2
ERROR: InexactError()
Stacktrace:
 [1] convert(::Type{Int64}, ::Float64) at ./float.jl:679
 [2] setindex!(::Array{Int64,1}, ::Float64, ::Int64) at ./array.jl:583

Though that might be only a limitation of the current implementation.

Initially I thought it would be very useful to have @set mutate when possible to easier write generic code that does the right thing on both immutable and mutable objects. Now I am not so sure anymore. I had a couple of situations where I had a nested struct with a buried array somewhere and I wanted a modified copy instead of changing the original.

jw3126 commented 6 years ago

I am leaning towards making @set be non mutating and adding an @set! macro. The difference between the two is that:

  1. @set! is mutating
  2. @set! overwrites the binding

For instance

@set! obj.field = val

will be equivalent to

let l = @lens _.field
    obj = set(l, obj, val, EncourageMutation())
end

while

@set obj.field = val

will be equivalent to

let l = @lens _.field
    set(l, obj, val, ForbidMutation())
end

What do you think @tkf ?

tkf commented 6 years ago

Yeah, I think that's good. I was thinking the same thing. One thing we need to mention in the document is that with @set! and static struct with type parameters, type instability may happen. But I think it's OK since it's usual Julia caution when you want performance.

jw3126 commented 6 years ago

Closed by #25, at least for now :smile: