jw3126 / Setfield.jl

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

Supporting @lens _[end] etc? #15

Closed tkf closed 5 years ago

tkf commented 6 years ago

Are you planning to support those cases?

l = @lens _[end]
@test get(l, [1, 2, 3]) == 3

l = @lens _[end-1]
@test get(l, [1, 2, 3]) == 2

l = @lens _[end-1][end]  # some complex case
@test get(l, [[1, 2, 3], []]) == 3

l = @lens _[:end]  # just make sure that symbol index is still ok
@test get(l, Dict(:end => 1)) == 1
tkf commented 6 years ago

Also with colons, like @lens _[end-2:end].

tkf commented 6 years ago

I actually looked at the code and tried to fix it. But then I realized that you need a little interpreter for the indexing operations involving the arithmetic operations and : (and what else?). You also need to be careful about what to interpret at the macro eval time; e.g., @lens _[f()-end] should eval f(), replace the result x with the original indexing AST, and store x-end. This is not a super hairy problem but requires some non-trivial code. Is it the only way to do it?

tkf commented 6 years ago

I had a quick try: #16. It turned out the indexing lenses lose type stability unless we make the indexing interpreter type stable as well. It means to infer the type of the expression without evaluating it when @lens is invoked. I guess it can be done by, e.g., injecting some dummy integer to end, eval the index expression, and store the type of the result as the type parameter of IndexLens. But I'm still not sure if it makes the indexing interpreter type stable.

tkf commented 6 years ago

Maybe more reasonable approach is to do the eval in @set and only allow concrete indices in IndexLens.

jw3126 commented 6 years ago

I would definitely like to support end (I think in future there will also be begin). However I don't really now how x[1:end-1] works internally etc. Does it get lowered to something like x[1:lastindex(x) - 1]?

jw3126 commented 6 years ago

Okay I think I have an idea how to implement this treating the 1:end mechanism as a blackbox.

struct OOPLens{S,G,M} <: Lens
    set::S
    get::G
    modify::M
end

and @lens _[fancy_index_expression] expands to

let 
    set(obj, val) = obj[fancy_index_expression] = val; obj
    ...
   OOPLens(set,get,modify)
end

One detail is, that obj[fancy_index_expression] = val involves setindex! and we might like setindex. We could however patch this by wrapping the object

struct SetindexWrapper{O}
    obj::O
end
setindex!(w::SetindexWrapper, args...) = SetindexWrapper(setindex(w.obj, args...)

and replace e.g

    set(obj, val) = obj[fancy_index_expression] = val; obj

by

    set(obj, val) = w = SetindexWrapper(obj); w[fancy_index_expression] = val; w.obj
tkf commented 6 years ago

Ooooh, this is much cleaner solution.