From 31be9d6c36db32515971c4704819ac9e53f52059 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sat, 27 Jul 2024 12:43:30 +0200 Subject: [PATCH 01/18] Use more generated functions for setproperties/getproperties --- src/ConstructionBase.jl | 67 ++++++++++++++++++++++++++++------------- test/runtests.jl | 11 +++++++ 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index ba31236..fc1c50a 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -45,14 +45,25 @@ end ################################################################################ getfields(x::Tuple) = x getfields(x::NamedTuple) = x +@generated function getfields(obj) + fnames = fieldnames(obj) + if eltype(fnames) === Int + :(getfield.((obj,), $fnames)) + else + :(NamedTuple{$fnames}(getfield.((obj,), $fnames))) + end +end + getproperties(o::NamedTuple) = o getproperties(o::Tuple) = o if VERSION >= v"1.7" + properties_are_fields(obj) = propertynames(obj) === fieldnames(typeof(obj)) + function check_properties_are_fields(obj) # for ntuples of symbols `===` is semantically the same as `==` # but triple equals is easier for the compiler to optimize, see #82 - if propertynames(obj) !== fieldnames(typeof(obj)) + if !properties_are_fields(obj) error(""" The function `Base.propertynames` was overloaded for type `$(typeof(obj))`. Please make sure `ConstructionBase.setproperties` is also overloaded for this type. @@ -60,12 +71,15 @@ if VERSION >= v"1.7" end end else + properties_are_fields(obj) = properties_are_fields(typeof(obj)) + properties_are_fields(T::Type) = is_propertynames_overloaded(T) + function is_propertynames_overloaded(T::Type)::Bool which(propertynames, Tuple{T}).sig !== Tuple{typeof(propertynames), Any} end @generated function check_properties_are_fields(obj) - if is_propertynames_overloaded(obj) + if !properties_are_fields(obj) return quote T = typeof(obj) msg = """ @@ -101,21 +115,14 @@ tuple_or_ntuple(::Type, names, vals) = error("Only Int and Symbol propertynames if VERSION >= v"1.7" function getproperties(obj) - fnames = propertynames(obj) - tuple_or_ntuple(fnames, getproperty.((obj,), fnames)) - end - function getfields(obj::T) where {T} - fnames = fieldnames(T) - NamedTuple{fnames}(getfield.((obj,), fnames)) - end -else - @generated function getfields(obj) - fnames = fieldnames(obj) - fvals = map(fnames) do fname - Expr(:call, :getfield, :obj, QuoteNode(fname)) + if properties_are_fields(obj) + getfields(obj) + else + fnames = propertynames(obj) + tuple_or_ntuple(fnames, getproperty.((obj,), fnames)) end - :(NamedTuple{$fnames}(($(fvals...),))) end +else function getproperties(obj) check_properties_are_fields(obj) getfields(obj) @@ -150,12 +157,23 @@ function setproperties_namedtuple(obj, patch) check_patch_properties_exist(res, obj, obj, patch) res end +function check_patch_fields_exist(obj, patch) + fnames = fieldnames(typeof(obj)) + pnames = fieldnames(typeof(patch)) + for pname in pnames + if !in(pname, fnames) + msg = """ + Failed to assign fields $pnames to object with fields $fnames. + """ + throw(ArgumentError(msg)) + end + end +end function check_patch_properties_exist( nt_new::NamedTuple{fields}, nt_old::NamedTuple{fields}, obj, patch) where {fields} nothing end @noinline function check_patch_properties_exist(nt_new, nt_old, obj, patch) - O = typeof(obj) msg = """ Failed to assign properties $(propertynames(patch)) to object with properties $(propertynames(obj)). """ @@ -210,13 +228,20 @@ setproperties_object(obj, patch::Tuple{}) = obj end setproperties_object(obj, patch::NamedTuple{()}) = obj +@generated function setfields_object(obj, patch) + args = Expr[] + pnames = fieldnames(patch) + for fname in fieldnames(obj) + source = fname in pnames ? :patch : :obj + push!(args, :(getfield($source, $(QuoteNode(fname))))) + end + :(constructorof(typeof(obj))($(args...))) +end + function setproperties_object(obj, patch) check_properties_are_fields(obj) - nt = getproperties(obj) - nt_new = merge(nt, patch) - check_patch_properties_exist(nt_new, nt, obj, patch) - args = Tuple(nt_new) # old julia inference prefers if we wrap in Tuple - constructorof(typeof(obj))(args...) + check_patch_fields_exist(obj, patch) + setfields_object(obj, patch) end include("nonstandard.jl") diff --git a/test/runtests.jl b/test/runtests.jl index f4e68ff..57f9acc 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -476,6 +476,17 @@ end end end +struct S2 + a::Union{Nothing, Int} + b::Union{UInt32, Int32} +end + +@testset "no allocs S2" begin + obj = S2(3, UInt32(5)) + @test 0 == hot_loop_allocs(constructorof, typeof(obj)) + @test 0 == hot_loop_allocs(setproperties, obj, (; a = nothing, b = Int32(6))) +end + @testset "inference" begin @testset "Tuple n=$n" for n in [0,1,2,3,4,5,10,20,30,40] t = funny_numbers(Tuple,n) From f4cfd34c3989b648dcd93ab84817243d394ba09d Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sat, 27 Jul 2024 13:05:06 +0200 Subject: [PATCH 02/18] Fix properties_are_fields --- src/ConstructionBase.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index fc1c50a..38e937c 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -72,7 +72,7 @@ if VERSION >= v"1.7" end else properties_are_fields(obj) = properties_are_fields(typeof(obj)) - properties_are_fields(T::Type) = is_propertynames_overloaded(T) + properties_are_fields(T::Type) = !is_propertynames_overloaded(T) function is_propertynames_overloaded(T::Type)::Bool which(propertynames, Tuple{T}).sig !== Tuple{typeof(propertynames), Any} From ac65fcddc54463db177414dde287b711488dfdb1 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sat, 27 Jul 2024 13:28:38 +0200 Subject: [PATCH 03/18] Avoid allocations on older Julia versions --- src/ConstructionBase.jl | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index 38e937c..ce509b5 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -47,11 +47,11 @@ getfields(x::Tuple) = x getfields(x::NamedTuple) = x @generated function getfields(obj) fnames = fieldnames(obj) - if eltype(fnames) === Int - :(getfield.((obj,), $fnames)) - else - :(NamedTuple{$fnames}(getfield.((obj,), $fnames))) + tuple = Expr(:tuple) + for fname in fnames + push!(tuple.args, :(getfield(obj, $(QuoteNode(fname))))) end + eltype(fnames) === Int ? tuple : :(NamedTuple{$fnames}($tuple)) end getproperties(o::NamedTuple) = o @@ -157,17 +157,18 @@ function setproperties_namedtuple(obj, patch) check_patch_properties_exist(res, obj, obj, patch) res end -function check_patch_fields_exist(obj, patch) - fnames = fieldnames(typeof(obj)) - pnames = fieldnames(typeof(patch)) +@generated function check_patch_fields_exist(obj, patch) + fnames = fieldnames(obj) + pnames = fieldnames(patch) for pname in pnames if !in(pname, fnames) msg = """ Failed to assign fields $pnames to object with fields $fnames. """ - throw(ArgumentError(msg)) + return :(throw(ArgumentError($msg))) end end + :(nothing) end function check_patch_properties_exist( nt_new::NamedTuple{fields}, nt_old::NamedTuple{fields}, obj, patch) where {fields} From 812884d32fa1b9bc737dbe2be4199ccd8ca32c52 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sat, 27 Jul 2024 13:38:24 +0200 Subject: [PATCH 04/18] Allow allocs for union types on older Julia versions --- src/ConstructionBase.jl | 2 +- test/runtests.jl | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index ce509b5..2666564 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -234,7 +234,7 @@ setproperties_object(obj, patch::NamedTuple{()}) = obj pnames = fieldnames(patch) for fname in fieldnames(obj) source = fname in pnames ? :patch : :obj - push!(args, :(getfield($source, $(QuoteNode(fname))))) + push!(args, :(getproperty($source, $(QuoteNode(fname))))) end :(constructorof(typeof(obj))($(args...))) end diff --git a/test/runtests.jl b/test/runtests.jl index 57f9acc..0888c6b 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -484,7 +484,11 @@ end @testset "no allocs S2" begin obj = S2(3, UInt32(5)) @test 0 == hot_loop_allocs(constructorof, typeof(obj)) - @test 0 == hot_loop_allocs(setproperties, obj, (; a = nothing, b = Int32(6))) + if VERSION ≤ v"1.3" + @test 32 == hot_loop_allocs(setproperties, obj, (; a = nothing, b = Int32(6))) + else + @test 0 == hot_loop_allocs(setproperties, obj, (; a = nothing, b = Int32(6))) + end end @testset "inference" begin From 20fc674a0f8b1d51c1c32d9b4379df6914953664 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sat, 27 Jul 2024 13:48:04 +0200 Subject: [PATCH 05/18] Adjust test bound --- test/runtests.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 0888c6b..6e5fff9 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -484,8 +484,8 @@ end @testset "no allocs S2" begin obj = S2(3, UInt32(5)) @test 0 == hot_loop_allocs(constructorof, typeof(obj)) - if VERSION ≤ v"1.3" - @test 32 == hot_loop_allocs(setproperties, obj, (; a = nothing, b = Int32(6))) + if VERSION < v"1.6" + @test 32 ≥ hot_loop_allocs(setproperties, obj, (; a = nothing, b = Int32(6))) else @test 0 == hot_loop_allocs(setproperties, obj, (; a = nothing, b = Int32(6))) end From c73435f8207e90aa7c4807adfe4115af1248ec7d Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sun, 28 Jul 2024 14:16:03 +0200 Subject: [PATCH 06/18] Annotate patch with NamedTuple --- src/ConstructionBase.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index 2666564..95b288f 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -239,7 +239,7 @@ setproperties_object(obj, patch::NamedTuple{()}) = obj :(constructorof(typeof(obj))($(args...))) end -function setproperties_object(obj, patch) +function setproperties_object(obj, patch::NamedTuple) check_properties_are_fields(obj) check_patch_fields_exist(obj, patch) setfields_object(obj, patch) From 9e83da33056d61738c483bd089685580a1a664b4 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sun, 28 Jul 2024 14:16:46 +0200 Subject: [PATCH 07/18] Attempt to remove method on ::Type --- src/ConstructionBase.jl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index 95b288f..b7a4366 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -71,15 +71,14 @@ if VERSION >= v"1.7" end end else - properties_are_fields(obj) = properties_are_fields(typeof(obj)) - properties_are_fields(T::Type) = !is_propertynames_overloaded(T) + properties_are_fields(obj) = is_propertynames_overloaded(typeof(obj)) function is_propertynames_overloaded(T::Type)::Bool which(propertynames, Tuple{T}).sig !== Tuple{typeof(propertynames), Any} end @generated function check_properties_are_fields(obj) - if !properties_are_fields(obj) + if is_propertynames_overloaded(obj) return quote T = typeof(obj) msg = """ From 1b8840b83ffd1e8676398d0ae49ef0b70268e12b Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sun, 28 Jul 2024 14:19:09 +0200 Subject: [PATCH 08/18] Reuse old getfields implementation --- src/ConstructionBase.jl | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index b7a4366..7207e2b 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -47,11 +47,10 @@ getfields(x::Tuple) = x getfields(x::NamedTuple) = x @generated function getfields(obj) fnames = fieldnames(obj) - tuple = Expr(:tuple) - for fname in fnames - push!(tuple.args, :(getfield(obj, $(QuoteNode(fname))))) + fvals = map(fnames) do fname + Expr(:call, :getfield, :obj, QuoteNode(fname)) end - eltype(fnames) === Int ? tuple : :(NamedTuple{$fnames}($tuple)) + :(NamedTuple{$fnames}(($(fvals...),))) end getproperties(o::NamedTuple) = o From d13e05f05daf7de990fd78e6619cee15e41179f0 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sun, 28 Jul 2024 14:31:50 +0200 Subject: [PATCH 09/18] Remove `setproperties_namedtuple` dispatch --- src/ConstructionBase.jl | 29 ++--------------------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index 7207e2b..5abaee1 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -136,25 +136,9 @@ end setproperties(obj , patch::Tuple ) = setproperties_object(obj , patch ) setproperties(obj , patch::NamedTuple ) = setproperties_object(obj , patch ) -setproperties(obj::NamedTuple , patch::Tuple ) = setproperties_namedtuple(obj , patch ) -setproperties(obj::NamedTuple , patch::NamedTuple ) = setproperties_namedtuple(obj , patch ) setproperties(obj::Tuple , patch::Tuple ) = setproperties_tuple(obj , patch ) setproperties(obj::Tuple , patch::NamedTuple ) = setproperties_tuple(obj , patch ) -setproperties_namedtuple(obj, patch::Tuple{}) = obj -@noinline function setproperties_namedtuple(obj, patch::Tuple) - msg = """ - setproperties(obj::NamedTuple, patch::Tuple) only allowed for empty Tuple. Got: - obj = $obj - patch = $patch - """ - throw(ArgumentError(msg)) -end -function setproperties_namedtuple(obj, patch) - res = merge(obj, patch) - check_patch_properties_exist(res, obj, obj, patch) - res -end @generated function check_patch_fields_exist(obj, patch) fnames = fieldnames(obj) pnames = fieldnames(patch) @@ -168,17 +152,8 @@ end end :(nothing) end -function check_patch_properties_exist( - nt_new::NamedTuple{fields}, nt_old::NamedTuple{fields}, obj, patch) where {fields} - nothing -end -@noinline function check_patch_properties_exist(nt_new, nt_old, obj, patch) - msg = """ - Failed to assign properties $(propertynames(patch)) to object with properties $(propertynames(obj)). - """ - throw(ArgumentError(msg)) -end -function setproperties_namedtuple(obj::NamedTuple{fields}, patch::NamedTuple{fields}) where {fields} + +function setproperties(obj::NamedTuple{fields}, patch::NamedTuple{fields}) where {fields} patch end From 74223553ad8ae13094f71d1664df22a387a30baa Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sun, 28 Jul 2024 14:32:14 +0200 Subject: [PATCH 10/18] Add another NamedTuple annotation --- src/ConstructionBase.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index 5abaee1..172d088 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -202,7 +202,7 @@ setproperties_object(obj, patch::Tuple{}) = obj end setproperties_object(obj, patch::NamedTuple{()}) = obj -@generated function setfields_object(obj, patch) +@generated function setfields_object(obj, patch::NamedTuple) args = Expr[] pnames = fieldnames(patch) for fname in fieldnames(obj) From 10f40d94562b5a9a1fcf1409df5c6a8f195bd8b1 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sun, 28 Jul 2024 14:37:34 +0200 Subject: [PATCH 11/18] Fix `is_propertynames_overloaded` for NamedTuple --- src/ConstructionBase.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index 172d088..e0aa3f7 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -73,6 +73,7 @@ else properties_are_fields(obj) = is_propertynames_overloaded(typeof(obj)) function is_propertynames_overloaded(T::Type)::Bool + T <: NamedTuple && return false which(propertynames, Tuple{T}).sig !== Tuple{typeof(propertynames), Any} end From a7abddc522e655e57914cad5b9f417afe233e822 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Mon, 29 Jul 2024 12:46:59 +0200 Subject: [PATCH 12/18] Remove obsolete comment --- src/ConstructionBase.jl | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index e0aa3f7..4ddef12 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -60,8 +60,6 @@ if VERSION >= v"1.7" properties_are_fields(obj) = propertynames(obj) === fieldnames(typeof(obj)) function check_properties_are_fields(obj) - # for ntuples of symbols `===` is semantically the same as `==` - # but triple equals is easier for the compiler to optimize, see #82 if !properties_are_fields(obj) error(""" The function `Base.propertynames` was overloaded for type `$(typeof(obj))`. From d072e3f2dffb9dd3c5d3d6f301023cf28e9f1676 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Tue, 30 Jul 2024 20:48:51 +0200 Subject: [PATCH 13/18] Restore `getproperties` implementation --- src/ConstructionBase.jl | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index 4ddef12..fcc1972 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -45,22 +45,14 @@ end ################################################################################ getfields(x::Tuple) = x getfields(x::NamedTuple) = x -@generated function getfields(obj) - fnames = fieldnames(obj) - fvals = map(fnames) do fname - Expr(:call, :getfield, :obj, QuoteNode(fname)) - end - :(NamedTuple{$fnames}(($(fvals...),))) -end - getproperties(o::NamedTuple) = o getproperties(o::Tuple) = o if VERSION >= v"1.7" - properties_are_fields(obj) = propertynames(obj) === fieldnames(typeof(obj)) - function check_properties_are_fields(obj) - if !properties_are_fields(obj) + # for ntuples of symbols `===` is semantically the same as `==` + # but triple equals is easier for the compiler to optimize, see #82 + if propertynames(obj) !== fieldnames(typeof(obj)) error(""" The function `Base.propertynames` was overloaded for type `$(typeof(obj))`. Please make sure `ConstructionBase.setproperties` is also overloaded for this type. @@ -68,8 +60,6 @@ if VERSION >= v"1.7" end end else - properties_are_fields(obj) = is_propertynames_overloaded(typeof(obj)) - function is_propertynames_overloaded(T::Type)::Bool T <: NamedTuple && return false which(propertynames, Tuple{T}).sig !== Tuple{typeof(propertynames), Any} @@ -112,14 +102,21 @@ tuple_or_ntuple(::Type, names, vals) = error("Only Int and Symbol propertynames if VERSION >= v"1.7" function getproperties(obj) - if properties_are_fields(obj) - getfields(obj) - else - fnames = propertynames(obj) - tuple_or_ntuple(fnames, getproperty.((obj,), fnames)) - end + fnames = propertynames(obj) + tuple_or_ntuple(fnames, getproperty.((obj,), fnames)) + end + function getfields(obj::T) where {T} + fnames = fieldnames(T) + NamedTuple{fnames}(getfield.((obj,), fnames)) end else + @generated function getfields(obj) + fnames = fieldnames(obj) + fvals = map(fnames) do fname + Expr(:call, :getfield, :obj, QuoteNode(fname)) + end + :(NamedTuple{$fnames}(($(fvals...),))) + end function getproperties(obj) check_properties_are_fields(obj) getfields(obj) From 69662e780ab2dba8aea24f2cc631681a965ae584 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Wed, 31 Jul 2024 14:14:34 +0200 Subject: [PATCH 14/18] Encourage union splitting during `NamedTuple` creation. --- src/ConstructionBase.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index fcc1972..1e25753 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -96,7 +96,8 @@ end Tuple(vals) end # names are symbols: return namedtuple -@inline tuple_or_ntuple(::Type{Symbol}, names, vals) = NamedTuple{Tuple(names)}(vals) +@inline tuple_or_ntuple(::Type{Symbol}, names, vals) = isa(vals, Tuple) ? namedtuple(names, vals...) : NamedTuple{Tuple(names)}(vals) +@inline namedtuple(names, vals...) = NamedTuple{Tuple(names)}((vals...,)) # this seemingly unnecessary method encourages union splitting. # otherwise: throw an error tuple_or_ntuple(::Type, names, vals) = error("Only Int and Symbol propertynames are supported") From 8ea24ec514b550974f19e36c0f5cba9cfab3b503 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Tue, 13 Aug 2024 12:47:25 +0200 Subject: [PATCH 15/18] Turn Tuple check into an additional method --- src/ConstructionBase.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index 1e25753..59bb7d8 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -96,7 +96,8 @@ end Tuple(vals) end # names are symbols: return namedtuple -@inline tuple_or_ntuple(::Type{Symbol}, names, vals) = isa(vals, Tuple) ? namedtuple(names, vals...) : NamedTuple{Tuple(names)}(vals) +@inline tuple_or_ntuple(::Type{Symbol}, names, vals::Tuple) = namedtuple(names, vals...) +@inline tuple_or_ntuple(::Type{Symbol}, names, vals) = NamedTuple{Tuple(names)}(vals) @inline namedtuple(names, vals...) = NamedTuple{Tuple(names)}((vals...,)) # this seemingly unnecessary method encourages union splitting. # otherwise: throw an error tuple_or_ntuple(::Type, names, vals) = error("Only Int and Symbol propertynames are supported") From b711a1c242329ce9e7fabc32ae02806216a7327d Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sun, 18 Aug 2024 22:25:17 +0200 Subject: [PATCH 16/18] Make check more concise --- src/ConstructionBase.jl | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index 59bb7d8..e7b7b9b 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -140,15 +140,7 @@ setproperties(obj::Tuple , patch::NamedTuple ) = setproperties_tuple(obj @generated function check_patch_fields_exist(obj, patch) fnames = fieldnames(obj) pnames = fieldnames(patch) - for pname in pnames - if !in(pname, fnames) - msg = """ - Failed to assign fields $pnames to object with fields $fnames. - """ - return :(throw(ArgumentError($msg))) - end - end - :(nothing) + fnames ⊆ pnames ? :(nothing) : :(throw(ArgumentError($("Failed to assign fields $pnames to object with fields $fnames.")))) end function setproperties(obj::NamedTuple{fields}, patch::NamedTuple{fields}) where {fields} From 5fb5633725677dfa0d33604640895bbd90767743 Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sun, 18 Aug 2024 22:25:35 +0200 Subject: [PATCH 17/18] Bump version --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index aac34fb..da7b104 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "ConstructionBase" uuid = "187b0558-2788-49d3-abe0-74a17ed4e7c9" authors = ["Takafumi Arakaki", "Rafael Schouten", "Jan Weidner"] -version = "1.5.6" +version = "1.5.7" [deps] LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" From 94c6636ffd74d58dd1534ac34329165e441be5dc Mon Sep 17 00:00:00 2001 From: serenity4 Date: Sun, 18 Aug 2024 22:28:02 +0200 Subject: [PATCH 18/18] Whoops, check was backwards --- src/ConstructionBase.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ConstructionBase.jl b/src/ConstructionBase.jl index e7b7b9b..c0d1236 100644 --- a/src/ConstructionBase.jl +++ b/src/ConstructionBase.jl @@ -140,7 +140,7 @@ setproperties(obj::Tuple , patch::NamedTuple ) = setproperties_tuple(obj @generated function check_patch_fields_exist(obj, patch) fnames = fieldnames(obj) pnames = fieldnames(patch) - fnames ⊆ pnames ? :(nothing) : :(throw(ArgumentError($("Failed to assign fields $pnames to object with fields $fnames.")))) + pnames ⊆ fnames ? :(nothing) : :(throw(ArgumentError($("Failed to assign fields $pnames to object with fields $fnames.")))) end function setproperties(obj::NamedTuple{fields}, patch::NamedTuple{fields}) where {fields}