From b346588d5e9a8e5b3c98a615da2e9c49647f35ce Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 21 Sep 2019 18:44:00 -0700 Subject: [PATCH 01/11] Support indexing based involving end --- Project.toml | 1 + src/Setfield.jl | 3 ++- src/lens.jl | 9 +++++++++ src/sugar.jl | 19 +++++++++++++++++++ 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index 39391ef..70889e9 100644 --- a/Project.toml +++ b/Project.toml @@ -3,6 +3,7 @@ uuid = "efcf1570-3423-57d1-acb7-fd33fddbac46" version = "0.4.1" [deps] +GroundEffects = "d00d4687-65cd-4fba-8fd9-ecb45a036ebe" MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09" [compat] diff --git a/src/Setfield.jl b/src/Setfield.jl index 62e5f1e..f9be3b2 100644 --- a/src/Setfield.jl +++ b/src/Setfield.jl @@ -1,7 +1,8 @@ __precompile__(true) module Setfield +using GroundEffects using MacroTools -using MacroTools: isstructdef, splitstructdef +using MacroTools: isstructdef, splitstructdef, postwalk include("lens.jl") include("sugar.jl") diff --git a/src/lens.jl b/src/lens.jl index 17d5968..760169b 100644 --- a/src/lens.jl +++ b/src/lens.jl @@ -278,6 +278,15 @@ Base.@propagate_inbounds set(obj, ::ConstIndexLens{I}, val) where I = end end +struct DynamicIndexLens{F} <: Lens + f::F +end + +Base.@propagate_inbounds get(obj, I::DynamicIndexLens) = obj[I.f(obj)...] + +Base.@propagate_inbounds set(obj, I::DynamicIndexLens, val) = + setindex(obj, val, I.f(obj)...) + """ FunctionLens(f) @lens f(_) diff --git a/src/sugar.jl b/src/sugar.jl index dee7bb6..8e907a7 100644 --- a/src/sugar.jl +++ b/src/sugar.jl @@ -52,6 +52,17 @@ end is_interpolation(x) = x isa Expr && x.head == :$ +foldtree(op, init, x) = op(init, x) +foldtree(op, init, ex::Expr) = + op(foldl((acc, x) -> foldtree(op, acc, x), ex.args; init=init), ex) + +need_dynamic_lens(ex) = + foldtree(false, ex) do yes, x + yes || x === :end || x === :_ + end + +replace_underscore(ex, to) = postwalk(x -> x === :_ ? to : x, ex) + function parse_obj_lenses(ex) if @capture(ex, front_[indices__]) obj, frontlens = parse_obj_lenses(front) @@ -63,6 +74,14 @@ function parse_obj_lenses(ex) end index = esc(Expr(:tuple, [x.args[1] for x in indices]...)) lens = :(ConstIndexLens{$index}()) + elseif obj == esc(:_) && any(need_dynamic_lens, indices) + @gensym collection + indices = replace_underscore.(indices, collection) + lex = GroundEffects.lower(:($collection[$(indices...)])) + @assert lex.args[1] == getindex + @assert lex.args[2] == collection + lindices = esc.(lex.args[3:end]) + lens = :(DynamicIndexLens($(esc(collection)) -> ($(lindices...),))) else index = esc(Expr(:tuple, indices...)) lens = :(IndexLens($index)) From 2b8c79b7a7fb19f8a2efbd5ee1f7bd4646d6adee Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 1 Oct 2019 18:16:39 -0700 Subject: [PATCH 02/11] Move lowering code needed here from GroundEffects --- Project.toml | 1 - src/Setfield.jl | 2 +- src/lowering.jl | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ src/sugar.jl | 2 +- 4 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 src/lowering.jl diff --git a/Project.toml b/Project.toml index 70889e9..39391ef 100644 --- a/Project.toml +++ b/Project.toml @@ -3,7 +3,6 @@ uuid = "efcf1570-3423-57d1-acb7-fd33fddbac46" version = "0.4.1" [deps] -GroundEffects = "d00d4687-65cd-4fba-8fd9-ecb45a036ebe" MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09" [compat] diff --git a/src/Setfield.jl b/src/Setfield.jl index f9be3b2..19436b9 100644 --- a/src/Setfield.jl +++ b/src/Setfield.jl @@ -1,9 +1,9 @@ __precompile__(true) module Setfield -using GroundEffects using MacroTools using MacroTools: isstructdef, splitstructdef, postwalk +include("lowering.jl") include("lens.jl") include("sugar.jl") include("functionlenses.jl") diff --git a/src/lowering.jl b/src/lowering.jl new file mode 100644 index 0000000..ba5f5ae --- /dev/null +++ b/src/lowering.jl @@ -0,0 +1,88 @@ +module Lowering + +using Base.Meta: isexpr + +""" + lower(ex) + +A modified version of `GroundEffects.lower` that only handles indexing. +""" +lower(ex) = something(handle_ref(lower, ex), ex) + +function handle_ref(lower, ex) + isexpr(ex, :ref) || return nothing + statements, collection, indices = _handle_ref(lower, ex) + push!(statements, Expr(:call, Base.getindex, collection, indices...)) + if length(statements) == 1 + return statements[1] + else + return Expr(:block, statements...) + end +end + +function _handle_ref(lower, ex) + statements = [] + if length(ex.args) == 1 + collection = ex.args[1] + indices = [] + else + if ex.args[1] isa Symbol + collection = ex.args[1] + else + @gensym collection + push!(statements, :($collection = $(ex.args[1]))) + end + indices = lower_indices(lower, collection, ex.args[2:end]) + end + return statements, collection, indices +end + +lower_indices(lower, collection, indices) = + map(index -> lower_index(lower, collection, index), indices) + +function lower_index(lower, collection, index) + ex = handle_dotcall(index) do ex + lower_index(lower, collection, ex) + end + ex === nothing || return something(ex) + + if isexpr(index, :call) + return Expr(:call, lower_indices(lower, collection, index.args)...) + elseif index === :end + return :($(Base.lastindex)($collection)) + end + return lower(index) +end + +function isdotopcall(ex) + ex isa Expr && !isempty(ex.args) || return false + op = ex.args[1] + return op isa Symbol && Base.isoperator(op) && startswith(string(op), ".") +end + +isdotcall(ex) = + isexpr(ex, :.) && length(ex.args) == 2 && isexpr(ex.args[2], :tuple) + +function handle_dotcall(lower, ex) + isdotcall(ex) || isdotopcall(ex) || return nothing + return Expr(:call, Base.materialize, handle_lazy_dotcall(lower, ex)) +end + +function handle_lazy_dotcall(lower, ex) + if isdotcall(ex) + args = [ + lower(ex.args[1]) + map(x -> handle_lazy_dotcall(lower, x), ex.args[2].args) + ] + elseif isdotopcall(ex) + args = [ + Symbol(String(ex.args[1])[2:end]) + map(x -> handle_lazy_dotcall(lower, x), ex.args[2:end]) + ] + else + return lower(ex) + end + return Expr(:call, Base.broadcasted, args...) +end + +end # module diff --git a/src/sugar.jl b/src/sugar.jl index 8e907a7..09d5dfe 100644 --- a/src/sugar.jl +++ b/src/sugar.jl @@ -77,7 +77,7 @@ function parse_obj_lenses(ex) elseif obj == esc(:_) && any(need_dynamic_lens, indices) @gensym collection indices = replace_underscore.(indices, collection) - lex = GroundEffects.lower(:($collection[$(indices...)])) + lex = Lowering.lower(:($collection[$(indices...)])) @assert lex.args[1] == getindex @assert lex.args[2] == collection lindices = esc.(lex.args[3:end]) From 6f7c35e230d180b3681cfd4302b93451e9eb9f3a Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 1 Oct 2019 18:25:57 -0700 Subject: [PATCH 03/11] Add tests --- test/test_core.jl | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/test_core.jl b/test/test_core.jl index 7f54dfc..4084ebd 100644 --- a/test/test_core.jl +++ b/test/test_core.jl @@ -104,6 +104,10 @@ end i = 1 si = @set t.a[i] = 10 @test s1 === si + se = @set t.a[end] = 20 + @test se === T((1,20),(3,4)) + se1 = @set t.a[end-1] = 10 + @test s1 === se1 s1 = @set t.a[$1] = 10 @test s1 === T((10,2),(3,4)) @@ -257,6 +261,18 @@ end @test get([4,5,6,7], l) == [4,5,6] end +@testset "DynamicIndexLens" begin + l = @lens _[end] + obj = (1,2,3) + @test get(obj, l) == 3 + @test set(obj, l, true) == (1,2,true) + + l = @lens _[end÷2] + obj = (1,2,3) + @test get(obj, l) == 1 + @test set(obj, l, true) == (true,2,3) +end + @testset "ConstIndexLens" begin obj = (1, 2.0, '3') l = @lens _[$1] From 8a3101bcb8b0dca2f1010f83aed58cc8758ecf22 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 1 Oct 2019 18:31:10 -0700 Subject: [PATCH 04/11] Make it work with set --- src/sugar.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sugar.jl b/src/sugar.jl index 09d5dfe..19b2bfe 100644 --- a/src/sugar.jl +++ b/src/sugar.jl @@ -74,7 +74,7 @@ function parse_obj_lenses(ex) end index = esc(Expr(:tuple, [x.args[1] for x in indices]...)) lens = :(ConstIndexLens{$index}()) - elseif obj == esc(:_) && any(need_dynamic_lens, indices) + elseif any(need_dynamic_lens, indices) @gensym collection indices = replace_underscore.(indices, collection) lex = Lowering.lower(:($collection[$(indices...)])) From 8da84a0db22a7b915a3b8c9ed5303fa39e1417a3 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 1 Oct 2019 18:36:59 -0700 Subject: [PATCH 05/11] Unsupport dot calls in index --- src/lowering.jl | 39 +++------------------------------------ 1 file changed, 3 insertions(+), 36 deletions(-) diff --git a/src/lowering.jl b/src/lowering.jl index ba5f5ae..95151a7 100644 --- a/src/lowering.jl +++ b/src/lowering.jl @@ -41,11 +41,9 @@ lower_indices(lower, collection, indices) = map(index -> lower_index(lower, collection, index), indices) function lower_index(lower, collection, index) - ex = handle_dotcall(index) do ex - lower_index(lower, collection, ex) - end - ex === nothing || return something(ex) - + # GroundEffects handles dot calls here but we don't need to do + # this because `setindex(::Tuple, ::Tuple, ::Tuple)` is not + # supported. if isexpr(index, :call) return Expr(:call, lower_indices(lower, collection, index.args)...) elseif index === :end @@ -54,35 +52,4 @@ function lower_index(lower, collection, index) return lower(index) end -function isdotopcall(ex) - ex isa Expr && !isempty(ex.args) || return false - op = ex.args[1] - return op isa Symbol && Base.isoperator(op) && startswith(string(op), ".") -end - -isdotcall(ex) = - isexpr(ex, :.) && length(ex.args) == 2 && isexpr(ex.args[2], :tuple) - -function handle_dotcall(lower, ex) - isdotcall(ex) || isdotopcall(ex) || return nothing - return Expr(:call, Base.materialize, handle_lazy_dotcall(lower, ex)) -end - -function handle_lazy_dotcall(lower, ex) - if isdotcall(ex) - args = [ - lower(ex.args[1]) - map(x -> handle_lazy_dotcall(lower, x), ex.args[2].args) - ] - elseif isdotopcall(ex) - args = [ - Symbol(String(ex.args[1])[2:end]) - map(x -> handle_lazy_dotcall(lower, x), ex.args[2:end]) - ] - else - return lower(ex) - end - return Expr(:call, Base.broadcasted, args...) -end - end # module From 7d7ba154ba79f81fc1ecb1dd8b96f3100b7efd4a Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 1 Oct 2019 18:44:18 -0700 Subject: [PATCH 06/11] Simplify src/lowering.jl using the assumption of Setfield usage --- src/lowering.jl | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/src/lowering.jl b/src/lowering.jl index 95151a7..80193a2 100644 --- a/src/lowering.jl +++ b/src/lowering.jl @@ -11,30 +11,9 @@ lower(ex) = something(handle_ref(lower, ex), ex) function handle_ref(lower, ex) isexpr(ex, :ref) || return nothing - statements, collection, indices = _handle_ref(lower, ex) - push!(statements, Expr(:call, Base.getindex, collection, indices...)) - if length(statements) == 1 - return statements[1] - else - return Expr(:block, statements...) - end -end - -function _handle_ref(lower, ex) - statements = [] - if length(ex.args) == 1 - collection = ex.args[1] - indices = [] - else - if ex.args[1] isa Symbol - collection = ex.args[1] - else - @gensym collection - push!(statements, :($collection = $(ex.args[1]))) - end - indices = lower_indices(lower, collection, ex.args[2:end]) - end - return statements, collection, indices + collection = ex.args[1] :: Symbol + indices = lower_indices(lower, collection, ex.args[2:end]) + return Expr(:call, Base.getindex, collection, indices...) end lower_indices(lower, collection, indices) = From 683df6e9949d044f29025288712daf0b4be28a6a Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 1 Oct 2019 18:52:54 -0700 Subject: [PATCH 07/11] Avoid failure in documentation build --- src/lowering.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lowering.jl b/src/lowering.jl index 80193a2..f71c9c5 100644 --- a/src/lowering.jl +++ b/src/lowering.jl @@ -2,11 +2,11 @@ module Lowering using Base.Meta: isexpr -""" +#= lower(ex) A modified version of `GroundEffects.lower` that only handles indexing. -""" +=# lower(ex) = something(handle_ref(lower, ex), ex) function handle_ref(lower, ex) From 4264829cf54f1261793b24f2cea1ac832f1ba1ce Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 1 Oct 2019 19:02:50 -0700 Subject: [PATCH 08/11] Stop using GroundEffects.jl-compatible internal interface --- src/Setfield.jl | 1 - src/lowering.jl | 34 ---------------------------------- src/sugar.jl | 14 ++++++++++---- 3 files changed, 10 insertions(+), 39 deletions(-) delete mode 100644 src/lowering.jl diff --git a/src/Setfield.jl b/src/Setfield.jl index 19436b9..576f9cf 100644 --- a/src/Setfield.jl +++ b/src/Setfield.jl @@ -3,7 +3,6 @@ module Setfield using MacroTools using MacroTools: isstructdef, splitstructdef, postwalk -include("lowering.jl") include("lens.jl") include("sugar.jl") include("functionlenses.jl") diff --git a/src/lowering.jl b/src/lowering.jl deleted file mode 100644 index f71c9c5..0000000 --- a/src/lowering.jl +++ /dev/null @@ -1,34 +0,0 @@ -module Lowering - -using Base.Meta: isexpr - -#= - lower(ex) - -A modified version of `GroundEffects.lower` that only handles indexing. -=# -lower(ex) = something(handle_ref(lower, ex), ex) - -function handle_ref(lower, ex) - isexpr(ex, :ref) || return nothing - collection = ex.args[1] :: Symbol - indices = lower_indices(lower, collection, ex.args[2:end]) - return Expr(:call, Base.getindex, collection, indices...) -end - -lower_indices(lower, collection, indices) = - map(index -> lower_index(lower, collection, index), indices) - -function lower_index(lower, collection, index) - # GroundEffects handles dot calls here but we don't need to do - # this because `setindex(::Tuple, ::Tuple, ::Tuple)` is not - # supported. - if isexpr(index, :call) - return Expr(:call, lower_indices(lower, collection, index.args)...) - elseif index === :end - return :($(Base.lastindex)($collection)) - end - return lower(index) -end - -end # module diff --git a/src/sugar.jl b/src/sugar.jl index 19b2bfe..fff0166 100644 --- a/src/sugar.jl +++ b/src/sugar.jl @@ -63,6 +63,15 @@ need_dynamic_lens(ex) = replace_underscore(ex, to) = postwalk(x -> x === :_ ? to : x, ex) +function lower_index(collection::Symbol, index) + if isexpr(index, :call) + return Expr(:call, lower_index.(collection, index.args)...) + elseif index === :end + return :($(Base.lastindex)($collection)) + end + return index +end + function parse_obj_lenses(ex) if @capture(ex, front_[indices__]) obj, frontlens = parse_obj_lenses(front) @@ -77,10 +86,7 @@ function parse_obj_lenses(ex) elseif any(need_dynamic_lens, indices) @gensym collection indices = replace_underscore.(indices, collection) - lex = Lowering.lower(:($collection[$(indices...)])) - @assert lex.args[1] == getindex - @assert lex.args[2] == collection - lindices = esc.(lex.args[3:end]) + lindices = esc.(lower_index.(collection, indices)) lens = :(DynamicIndexLens($(esc(collection)) -> ($(lindices...),))) else index = esc(Expr(:tuple, indices...)) From c70d7a9c782227c61ade7e5bfbfe576729f7e931 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 1 Oct 2019 22:37:48 -0700 Subject: [PATCH 09/11] Add more tests --- test/test_core.jl | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/test_core.jl b/test/test_core.jl index 4084ebd..e4b43dc 100644 --- a/test/test_core.jl +++ b/test/test_core.jl @@ -195,6 +195,8 @@ end @lens _.b.a.b[i] @lens _.b.a.b[$2] @lens _.b.a.b[$i] + @lens _.b.a.b[end] + @lens _.b.a.b[identity(end) - 1] @lens _ ] val1, val2 = randn(2) @@ -230,6 +232,8 @@ end ((@lens _.b.a.b[$(i+1)]), 4 ), ((@lens _.b.a.b[$2] ), 4.0), ((@lens _.b.a.b[$(i+1)]), 4.0), + ((@lens _.b.a.b[end]), 4.0), + ((@lens _.b.a.b[end÷2+1]), 4.0), ((@lens _ ), obj), ((@lens _ ), :xy), (MultiPropertyLens((a=(@lens _), b=(@lens _))), (a=1, b=2)), @@ -271,6 +275,13 @@ end obj = (1,2,3) @test get(obj, l) == 1 @test set(obj, l, true) == (true,2,3) + + two = 2 + plusone(x) = x + 1 + l = @lens _.a[plusone(end) - two].b + obj = (a=(1, (a=10, b=20), 3), b=4) + @test get(obj, l) == 20 + @test set(obj, l, true) == (a=(1, (a=10, b=true), 3), b=4) end @testset "ConstIndexLens" begin From de98e801e6a97b2d757ff31b60880360cdf7308a Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Tue, 1 Oct 2019 23:01:59 -0700 Subject: [PATCH 10/11] Support multi-dimensional DynamicIndexLens --- src/sugar.jl | 13 +++++++++---- test/test_staticarrays.jl | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/sugar.jl b/src/sugar.jl index fff0166..d735047 100644 --- a/src/sugar.jl +++ b/src/sugar.jl @@ -63,11 +63,15 @@ need_dynamic_lens(ex) = replace_underscore(ex, to) = postwalk(x -> x === :_ ? to : x, ex) -function lower_index(collection::Symbol, index) +function lower_index(collection::Symbol, index, dim) if isexpr(index, :call) - return Expr(:call, lower_index.(collection, index.args)...) + return Expr(:call, lower_index.(collection, index.args, dim)...) elseif index === :end - return :($(Base.lastindex)($collection)) + if dim === nothing + return :($(Base.lastindex)($collection)) + else + return :($(Base.lastindex)($collection, $dim)) + end end return index end @@ -86,7 +90,8 @@ function parse_obj_lenses(ex) elseif any(need_dynamic_lens, indices) @gensym collection indices = replace_underscore.(indices, collection) - lindices = esc.(lower_index.(collection, indices)) + dims = length(indices) == 1 ? nothing : 1:length(indices) + lindices = esc.(lower_index.(collection, indices, dims)) lens = :(DynamicIndexLens($(esc(collection)) -> ($(lindices...),))) else index = esc(Expr(:tuple, indices...)) diff --git a/test/test_staticarrays.jl b/test/test_staticarrays.jl index 4ec707c..ea9d3e7 100644 --- a/test/test_staticarrays.jl +++ b/test/test_staticarrays.jl @@ -17,5 +17,25 @@ using StaticArrays v = @SVector [1,2,3] @test (@set v[1] = 10) === @SVector [10,2,3] @test_broken (@set v[1] = π) === @SVector [π,2,3] + + @testset "Multi-dynamic indexing" begin + two = 2 + plusone(x) = x + 1 + l1 = @lens _.a[2, 1].b + l2 = @lens _.a[plusone(end) - two, end÷2].b + m_orig = @SMatrix [ + (a=1, b=10) (a=2, b=20) + (a=3, b=30) (a=4, b=40) + (a=5, b=50) (a=6, b=60) + ] + m_mod = @SMatrix [ + (a=1, b=10) (a=2, b=20) + (a=3, b=3000) (a=4, b=40) + (a=5, b=50) (a=6, b=60) + ] + obj = (a=m_orig, b=4) + @test get(obj, l1) === get(obj, l2) === 30 + @test set(obj, l1, 3000) === set(obj, l2, 3000) === (a=m_mod, b=4) + end end end From 6c7b02aecaf36ece3ce3862d36c0105a7ae878de Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Thu, 3 Oct 2019 13:17:52 -0700 Subject: [PATCH 11/11] Check index lens types in the tests --- test/test_core.jl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/test_core.jl b/test/test_core.jl index e4b43dc..6c2c532 100644 --- a/test/test_core.jl +++ b/test/test_core.jl @@ -246,32 +246,39 @@ end @testset "IndexLens" begin l = @lens _[] + @test l isa Setfield.IndexLens x = randn() obj = Ref(x) @test get(obj, l) == x l = @lens _[][] + @test l.outer isa Setfield.IndexLens + @test l.inner isa Setfield.IndexLens inner = Ref(x) obj = Base.RefValue{typeof(inner)}(inner) @test get(obj, l) == x obj = (1,2,3) l = @lens _[1] + @test l isa Setfield.IndexLens @test get(obj, l) == 1 @test set(obj, l, 6) == (6,2,3) l = @lens _[1:3] + @test l isa Setfield.IndexLens @test get([4,5,6,7], l) == [4,5,6] end @testset "DynamicIndexLens" begin l = @lens _[end] + @test l isa Setfield.DynamicIndexLens obj = (1,2,3) @test get(obj, l) == 3 @test set(obj, l, true) == (1,2,true) l = @lens _[end÷2] + @test l isa Setfield.DynamicIndexLens obj = (1,2,3) @test get(obj, l) == 1 @test set(obj, l, true) == (true,2,3)