From 7ef92aab3665b00368b6436e94829793622c2309 Mon Sep 17 00:00:00 2001 From: Hong Ge <3279477+yebai@users.noreply.github.com> Date: Sun, 31 Mar 2024 17:40:25 +0100 Subject: [PATCH 1/7] Update varname.jl --- src/varname.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/varname.jl b/src/varname.jl index a3ca1dc..e6ce3df 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -1,6 +1,5 @@ using Accessors using Accessors: ComposedOptic, PropertyLens, IndexLens, DynamicIndexLens -using MacroTools const ALLOWED_OPTICS = Union{typeof(identity),PropertyLens,IndexLens,ComposedOptic} From 45b0c33bc88b51748f109df94ceb105da7709115 Mon Sep 17 00:00:00 2001 From: Hong Ge <3279477+yebai@users.noreply.github.com> Date: Sun, 31 Mar 2024 17:40:57 +0100 Subject: [PATCH 2/7] Update Project.toml --- Project.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Project.toml b/Project.toml index becfdbe..d5fe1a2 100644 --- a/Project.toml +++ b/Project.toml @@ -9,7 +9,6 @@ version = "0.8.0" AbstractMCMC = "80f14c24-f653-4e6a-9b94-39d6b0f70001" Accessors = "7d9f7c33-5ae7-4f3b-8dc6-eff91059b697" DensityInterface = "b429d917-457f-4dbc-8f4c-0cc954292b1d" -MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09" Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" [compat] From 443656cb2b9464e37139c9ca9f4ec9646f80513c Mon Sep 17 00:00:00 2001 From: Xianda Sun Date: Sun, 31 Mar 2024 17:52:48 +0100 Subject: [PATCH 3/7] use `MacroTools` from `Accessors` --- src/varname.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/varname.jl b/src/varname.jl index e6ce3df..2035809 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -1,5 +1,6 @@ using Accessors using Accessors: ComposedOptic, PropertyLens, IndexLens, DynamicIndexLens +using Accessors.MacroTools: @capture const ALLOWED_OPTICS = Union{typeof(identity),PropertyLens,IndexLens,ComposedOptic} From 4850ef4ec0528c639129b0d8b1b1e6b8bb17452f Mon Sep 17 00:00:00 2001 From: Xianda Sun Date: Sun, 31 Mar 2024 18:42:02 +0100 Subject: [PATCH 4/7] stop using `MacroTools.@capture` --- src/varname.jl | 11 +++++++---- test/varname.jl | 10 +++++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index 2035809..598f048 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -1,6 +1,5 @@ using Accessors using Accessors: ComposedOptic, PropertyLens, IndexLens, DynamicIndexLens -using Accessors.MacroTools: @capture const ALLOWED_OPTICS = Union{typeof(identity),PropertyLens,IndexLens,ComposedOptic} @@ -689,12 +688,14 @@ function _parse_obj_optics_composite(lensexprs::Vector) end function _parse_obj_optics(ex) - if @capture(ex, ∘(opticsexprs__)) + if Meta.isexpr(ex, :call) && ex.args[1] == :∘ # matching ∘(opticsexprs) + opticsexprs = ex.args[2:end] return _parse_obj_optics_composite(opticsexprs) elseif is_interpolation(ex) @assert length(ex.args) == 1 return esc(:_), (esc(ex.args[1]),) - elseif @capture(ex, front_[indices__]) + elseif Meta.isexpr(ex, :ref) + front, indices... = ex.args obj, frontoptics = _parse_obj_optics(front) if any(Accessors.need_dynamic_optic, indices) @gensym collection @@ -706,7 +707,9 @@ function _parse_obj_optics(ex) index = esc(Expr(:tuple, indices...)) optics = :($(Accessors.IndexLens)($index)) end - elseif @capture(ex, front_.property_) + elseif Meta.isexpr(ex, :.) + front = ex.args[1] + property = ex.args[2].value # ex.args[2] is a QuoteNode obj, frontoptics = _parse_obj_optics(front) if property isa Union{Symbol,String} optics = :($(Accessors.PropertyLens){$(QuoteNode(property))}()) diff --git a/test/varname.jl b/test/varname.jl index d5cb4f7..d3f593f 100644 --- a/test/varname.jl +++ b/test/varname.jl @@ -89,7 +89,15 @@ end @test collect(get(B, @varname(B[1, :], true))) == collect(get(B, @varname(B[1, -4:5]))) end - + + @testset "optic expression parser" begin + ex = :(∘(_.f, _[1])) + obj, optics = AbstractPPL._parse_obj_optics(ex) + @test obj == :($(Expr(:escape, :_))) + @test optics[1] == (:(($(Accessors.PropertyLens)){:f}()),) + @test optics[2] == (:(($(Accessors.IndexLens))($(Expr(:escape, :((1,)))))),) + end + @testset "type stability" begin @inferred VarName{:a}() @inferred VarName{:a}(IndexLens(1)) From 9caa4dcef9d347217cf8b22e14024adc10b5376c Mon Sep 17 00:00:00 2001 From: Xianda Sun Date: Sun, 31 Mar 2024 18:44:51 +0100 Subject: [PATCH 5/7] rename test --- test/varname.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/varname.jl b/test/varname.jl index d3f593f..190da7d 100644 --- a/test/varname.jl +++ b/test/varname.jl @@ -90,7 +90,7 @@ end end - @testset "optic expression parser" begin + @testset "parsing composite optics" begin ex = :(∘(_.f, _[1])) obj, optics = AbstractPPL._parse_obj_optics(ex) @test obj == :($(Expr(:escape, :_))) From dca102098d82a160c37aa7dd6726a8cf29964679 Mon Sep 17 00:00:00 2001 From: Xianda Sun Date: Sun, 31 Mar 2024 18:57:55 +0100 Subject: [PATCH 6/7] remove support for composite lens --- src/varname.jl | 26 ++++---------------------- test/varname.jl | 8 -------- 2 files changed, 4 insertions(+), 30 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index 598f048..e8d3c88 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -670,28 +670,10 @@ function _parse_obj_optic(ex) obj, optic end -# Accessors doesn't have the same support for interpolation, so copy and modify Setfield's parsing functions -is_interpolation(x) = x isa Expr && x.head == :$ - -function _parse_obj_optics_composite(lensexprs::Vector) - if isempty(lensexprs) - return esc(:_), () - else - obj, outermostlens = _parse_obj_optics(lensexprs[1]) - innerlenses = map(lensexprs[2:end]) do innerex - o, lens = _parse_obj_optics(innerex) - @assert o == esc(:_) - lens - end - return obj, (outermostlens, innerlenses...) - end -end - +# Accessors doesn't have the same support for interpolation +# so this function is copied and altered from `Setfield._parse_obj_lens` function _parse_obj_optics(ex) - if Meta.isexpr(ex, :call) && ex.args[1] == :∘ # matching ∘(opticsexprs) - opticsexprs = ex.args[2:end] - return _parse_obj_optics_composite(opticsexprs) - elseif is_interpolation(ex) + if ex isa Expr && ex.head == :$ @assert length(ex.args) == 1 return esc(:_), (esc(ex.args[1]),) elseif Meta.isexpr(ex, :ref) @@ -713,7 +695,7 @@ function _parse_obj_optics(ex) obj, frontoptics = _parse_obj_optics(front) if property isa Union{Symbol,String} optics = :($(Accessors.PropertyLens){$(QuoteNode(property))}()) - elseif is_interpolation(property) + elseif property isa Expr && property.head == :$ optics = :($(Accessors.PropertyLens){$(esc(property.args[1]))}()) else throw(ArgumentError( diff --git a/test/varname.jl b/test/varname.jl index 190da7d..5087e8c 100644 --- a/test/varname.jl +++ b/test/varname.jl @@ -90,14 +90,6 @@ end end - @testset "parsing composite optics" begin - ex = :(∘(_.f, _[1])) - obj, optics = AbstractPPL._parse_obj_optics(ex) - @test obj == :($(Expr(:escape, :_))) - @test optics[1] == (:(($(Accessors.PropertyLens)){:f}()),) - @test optics[2] == (:(($(Accessors.IndexLens))($(Expr(:escape, :((1,)))))),) - end - @testset "type stability" begin @inferred VarName{:a}() @inferred VarName{:a}(IndexLens(1)) From 76ace06bf32dc7d41128cac5e3f88b3f84bf72a6 Mon Sep 17 00:00:00 2001 From: Xianda Sun <5433119+sunxd3@users.noreply.github.com> Date: Mon, 1 Apr 2024 06:42:47 +0100 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: David Widmann --- src/varname.jl | 9 ++++----- test/varname.jl | 1 - 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index e8d3c88..adcd568 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -673,10 +673,9 @@ end # Accessors doesn't have the same support for interpolation # so this function is copied and altered from `Setfield._parse_obj_lens` function _parse_obj_optics(ex) - if ex isa Expr && ex.head == :$ - @assert length(ex.args) == 1 + if Meta.isexpr(ex, :$, 1) return esc(:_), (esc(ex.args[1]),) - elseif Meta.isexpr(ex, :ref) + elseif Meta.isexpr(ex, :ref) && !isempty(ex.args) front, indices... = ex.args obj, frontoptics = _parse_obj_optics(front) if any(Accessors.need_dynamic_optic, indices) @@ -689,13 +688,13 @@ function _parse_obj_optics(ex) index = esc(Expr(:tuple, indices...)) optics = :($(Accessors.IndexLens)($index)) end - elseif Meta.isexpr(ex, :.) + elseif Meta.isexpr(ex, :., 2) front = ex.args[1] property = ex.args[2].value # ex.args[2] is a QuoteNode obj, frontoptics = _parse_obj_optics(front) if property isa Union{Symbol,String} optics = :($(Accessors.PropertyLens){$(QuoteNode(property))}()) - elseif property isa Expr && property.head == :$ + elseif Meta.isexpr(property, :$, 1) optics = :($(Accessors.PropertyLens){$(esc(property.args[1]))}()) else throw(ArgumentError( diff --git a/test/varname.jl b/test/varname.jl index 5087e8c..b117f0e 100644 --- a/test/varname.jl +++ b/test/varname.jl @@ -89,7 +89,6 @@ end @test collect(get(B, @varname(B[1, :], true))) == collect(get(B, @varname(B[1, -4:5]))) end - @testset "type stability" begin @inferred VarName{:a}() @inferred VarName{:a}(IndexLens(1))