From 3211be6837be78a1eab8231898c8a1b0a8795a4b Mon Sep 17 00:00:00 2001 From: Sukera Date: Wed, 6 Dec 2023 14:02:34 +0100 Subject: [PATCH 01/11] Define `==` for `Some` by forwarding to the wrapped value --- base/some.jl | 2 ++ test/some.jl | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/base/some.jl b/base/some.jl index 0d538cbed6c23..0f4d32f1fb537 100644 --- a/base/some.jl +++ b/base/some.jl @@ -145,3 +145,5 @@ macro something(args...) something = GlobalRef(Base, :something) return :($something($expr)) end + +==(a::Some, b::Some) = a.value == b.value diff --git a/test/some.jl b/test/some.jl index e49fc586a3a6e..358c683394275 100644 --- a/test/some.jl +++ b/test/some.jl @@ -49,6 +49,13 @@ @test !isequal(Some(1), nothing) @test !isequal(Some(nothing), nothing) +# Some with something else is false +@test !=(Some(nothing), nothing) +@test !=(nothing, Some(nothing)) + +# Two Somes forward to their wrapped things +@test ==(Some([0x1]), Some([1])) + @testset "something" begin @test_throws ArgumentError something() @test something(1) === 1 From 4b0532161c4ea61ae6529a0f436cab901559d55e Mon Sep 17 00:00:00 2001 From: Sukera Date: Wed, 6 Dec 2023 14:29:03 +0100 Subject: [PATCH 02/11] Implement hashing for custom `Some` equality --- base/some.jl | 1 + test/some.jl | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/base/some.jl b/base/some.jl index 0f4d32f1fb537..a91c3cd3bb5c1 100644 --- a/base/some.jl +++ b/base/some.jl @@ -147,3 +147,4 @@ macro something(args...) end ==(a::Some, b::Some) = a.value == b.value +hash(s::Some, h::UInt) = hash(s.value, hash(Some, h)) diff --git a/test/some.jl b/test/some.jl index 358c683394275..3c3050584061f 100644 --- a/test/some.jl +++ b/test/some.jl @@ -56,6 +56,10 @@ # Two Somes forward to their wrapped things @test ==(Some([0x1]), Some([1])) +# hashing implications +@test hash(Some(0x1)) != hash(0x1) +@test hash(Some(0x1)) == hash(Some(1)) + @testset "something" begin @test_throws ArgumentError something() @test something(1) === 1 From c122e73e7705e2417dad20e74a3e426fc3fd592f Mon Sep 17 00:00:00 2001 From: Sukera Date: Thu, 7 Dec 2023 23:16:16 +0100 Subject: [PATCH 03/11] Add missing support to `Some` equality --- base/some.jl | 1 + test/some.jl | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/base/some.jl b/base/some.jl index a91c3cd3bb5c1..37a1a50a1d48a 100644 --- a/base/some.jl +++ b/base/some.jl @@ -147,4 +147,5 @@ macro something(args...) end ==(a::Some, b::Some) = a.value == b.value +isequal(a::Some, b::Some) = isequal(a.value, b.value) hash(s::Some, h::UInt) = hash(s.value, hash(Some, h)) diff --git a/test/some.jl b/test/some.jl index 3c3050584061f..4e2c006b404b2 100644 --- a/test/some.jl +++ b/test/some.jl @@ -55,6 +55,12 @@ # Two Somes forward to their wrapped things @test ==(Some([0x1]), Some([1])) +@test ==(Some(1), missing) isa Missing +@test ==(missing, Some(1)) isa Missing +@test isequal(Some([0x1]), Some([1])) +@test !isequal(Some(missing), Some([1])) +@test !isequal(Some(1), Some(missing)) +@test isequal(Some(missing), Some(missing)) # hashing implications @test hash(Some(0x1)) != hash(0x1) From b6fbb4001029c4efe41cd2d78f0e8bafdddb475f Mon Sep 17 00:00:00 2001 From: Sukera Date: Thu, 7 Dec 2023 23:26:04 +0100 Subject: [PATCH 04/11] Ensure `==(::Some, ::Some{Missing})` doesn't unwrap --- base/some.jl | 2 ++ test/some.jl | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/base/some.jl b/base/some.jl index 37a1a50a1d48a..3a291c6a5131c 100644 --- a/base/some.jl +++ b/base/some.jl @@ -147,5 +147,7 @@ macro something(args...) end ==(a::Some, b::Some) = a.value == b.value +==(::Some{Missing}, ::Some{T}) where T = T === Missing +==(::Some{T}, ::Some{Missing}) where T = T === Missing isequal(a::Some, b::Some) = isequal(a.value, b.value) hash(s::Some, h::UInt) = hash(s.value, hash(Some, h)) diff --git a/test/some.jl b/test/some.jl index 4e2c006b404b2..39c1841ed2954 100644 --- a/test/some.jl +++ b/test/some.jl @@ -55,8 +55,15 @@ # Two Somes forward to their wrapped things @test ==(Some([0x1]), Some([1])) + +# Don't propagate wrapped missings +@test !=(Some(1), Some(missing)) +@test !=(Some(missing), Some(1)) + +# Make sure to still propagate non-wrapped Missing @test ==(Some(1), missing) isa Missing @test ==(missing, Some(1)) isa Missing + @test isequal(Some([0x1]), Some([1])) @test !isequal(Some(missing), Some([1])) @test !isequal(Some(1), Some(missing)) From 4db345619cfde58e9e1a2152cebd1636d2a23247 Mon Sep 17 00:00:00 2001 From: Sukera Date: Thu, 7 Dec 2023 23:57:31 +0100 Subject: [PATCH 05/11] Fix `Some{Missing}` ambiguity on equality --- base/some.jl | 2 ++ test/some.jl | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/base/some.jl b/base/some.jl index 3a291c6a5131c..89c94b749e4c6 100644 --- a/base/some.jl +++ b/base/some.jl @@ -149,5 +149,7 @@ end ==(a::Some, b::Some) = a.value == b.value ==(::Some{Missing}, ::Some{T}) where T = T === Missing ==(::Some{T}, ::Some{Missing}) where T = T === Missing +==(::Some{Missing}, ::Some{Missing}) = true + isequal(a::Some, b::Some) = isequal(a.value, b.value) hash(s::Some, h::UInt) = hash(s.value, hash(Some, h)) diff --git a/test/some.jl b/test/some.jl index 39c1841ed2954..04265607568b4 100644 --- a/test/some.jl +++ b/test/some.jl @@ -59,6 +59,7 @@ # Don't propagate wrapped missings @test !=(Some(1), Some(missing)) @test !=(Some(missing), Some(1)) +@test ==(Some(missing), Some(missing)) # Make sure to still propagate non-wrapped Missing @test ==(Some(1), missing) isa Missing @@ -69,6 +70,9 @@ @test !isequal(Some(1), Some(missing)) @test isequal(Some(missing), Some(missing)) +@test !isequal(missing, Some(missing)) +@test !isequal(Some(missing), missing) + # hashing implications @test hash(Some(0x1)) != hash(0x1) @test hash(Some(0x1)) == hash(Some(1)) From a8145d2eb205cadc7f6365a5c871524369bedecd Mon Sep 17 00:00:00 2001 From: Sukera <11753998+Seelengrab@users.noreply.github.com> Date: Fri, 8 Dec 2023 01:09:22 +0100 Subject: [PATCH 06/11] Remove unnecessary specialization Co-authored-by: Alex Arslan --- base/some.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/base/some.jl b/base/some.jl index 89c94b749e4c6..7fced13e30420 100644 --- a/base/some.jl +++ b/base/some.jl @@ -147,8 +147,8 @@ macro something(args...) end ==(a::Some, b::Some) = a.value == b.value -==(::Some{Missing}, ::Some{T}) where T = T === Missing -==(::Some{T}, ::Some{Missing}) where T = T === Missing +==(::Some{Missing}, ::Some) = false +==(::Some, ::Some{Missing}) = false ==(::Some{Missing}, ::Some{Missing}) = true isequal(a::Some, b::Some) = isequal(a.value, b.value) From 177625dba2601ce9dd8a544f6126f40030854c17 Mon Sep 17 00:00:00 2001 From: Sukera Date: Mon, 11 Dec 2023 01:45:09 +0100 Subject: [PATCH 07/11] Make `isequal` for `Some{Missing}` type stable --- base/some.jl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/base/some.jl b/base/some.jl index 7fced13e30420..cacdad2088f14 100644 --- a/base/some.jl +++ b/base/some.jl @@ -146,10 +146,14 @@ macro something(args...) return :($something($expr)) end -==(a::Some, b::Some) = a.value == b.value +==(a::Some, b::Some)::Bool = a.value == b.value ==(::Some{Missing}, ::Some) = false ==(::Some, ::Some{Missing}) = false ==(::Some{Missing}, ::Some{Missing}) = true -isequal(a::Some, b::Some) = isequal(a.value, b.value) +isequal(a::Some, b::Some)::Bool = isequal(a.value, b.value) +isequal(::Some{Missing}, ::Some) = false +isequal(::Some, ::Some{Missing}) = false +isequal(::Some{Missing}, ::Some{Missing}) = true + hash(s::Some, h::UInt) = hash(s.value, hash(Some, h)) From 28c6ecbc4a29bdd5b479546930e2e52551f4087d Mon Sep 17 00:00:00 2001 From: Sukera <11753998+Seelengrab@users.noreply.github.com> Date: Mon, 11 Dec 2023 09:54:15 +0100 Subject: [PATCH 08/11] Try to get around a false positive in the typo checker Co-authored-by: Alex Arslan --- test/some.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/some.jl b/test/some.jl index 04265607568b4..ed2775d9981de 100644 --- a/test/some.jl +++ b/test/some.jl @@ -53,10 +53,10 @@ @test !=(Some(nothing), nothing) @test !=(nothing, Some(nothing)) -# Two Somes forward to their wrapped things +# Two `Some`s forward to their wrapped things @test ==(Some([0x1]), Some([1])) -# Don't propagate wrapped missings +# Don't propagate wrapped `missing`s @test !=(Some(1), Some(missing)) @test !=(Some(missing), Some(1)) @test ==(Some(missing), Some(missing)) From 2b44260c81e10ea83ec485906dc8e90b7a894d5e Mon Sep 17 00:00:00 2001 From: Sukera Date: Tue, 2 Jan 2024 16:17:32 +0100 Subject: [PATCH 09/11] Remove specialized `missing` behavior --- base/some.jl | 11 ++++------- test/some.jl | 13 +++++-------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/base/some.jl b/base/some.jl index cacdad2088f14..15ddb1f8c037a 100644 --- a/base/some.jl +++ b/base/some.jl @@ -147,13 +147,10 @@ macro something(args...) end ==(a::Some, b::Some)::Bool = a.value == b.value -==(::Some{Missing}, ::Some) = false -==(::Some, ::Some{Missing}) = false -==(::Some{Missing}, ::Some{Missing}) = true +# seperated out to ensure the above assertion works +==(::Some{Missing}, ::Some) = missing +==(::Some, ::Some{Missing}) = missing +==(::Some{Missing}, ::Some{Missing}) = missing isequal(a::Some, b::Some)::Bool = isequal(a.value, b.value) -isequal(::Some{Missing}, ::Some) = false -isequal(::Some, ::Some{Missing}) = false -isequal(::Some{Missing}, ::Some{Missing}) = true - hash(s::Some, h::UInt) = hash(s.value, hash(Some, h)) diff --git a/test/some.jl b/test/some.jl index ed2775d9981de..064eaed1d6ecb 100644 --- a/test/some.jl +++ b/test/some.jl @@ -56,22 +56,19 @@ # Two `Some`s forward to their wrapped things @test ==(Some([0x1]), Some([1])) -# Don't propagate wrapped `missing`s -@test !=(Some(1), Some(missing)) -@test !=(Some(missing), Some(1)) -@test ==(Some(missing), Some(missing)) +# propagate wrapped missings +@test !=(Some(1), Some(missing)) isa Missing +@test !=(Some(missing), Some(1)) isa Missing +@test ==(Some(missing), Some(missing)) isa Missing # Make sure to still propagate non-wrapped Missing @test ==(Some(1), missing) isa Missing @test ==(missing, Some(1)) isa Missing @test isequal(Some([0x1]), Some([1])) -@test !isequal(Some(missing), Some([1])) -@test !isequal(Some(1), Some(missing)) -@test isequal(Some(missing), Some(missing)) - @test !isequal(missing, Some(missing)) @test !isequal(Some(missing), missing) +@test isequal(Some(missing), Some(missing)) # hashing implications @test hash(Some(0x1)) != hash(0x1) From e224a69f150a4e8aa29dbc07b4e9d5158f45e7f1 Mon Sep 17 00:00:00 2001 From: Sukera Date: Tue, 27 Feb 2024 09:10:08 +0100 Subject: [PATCH 10/11] Remove explicit handling of `Some{Missing}` --- base/some.jl | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/base/some.jl b/base/some.jl index 15ddb1f8c037a..3f95b04a45241 100644 --- a/base/some.jl +++ b/base/some.jl @@ -146,11 +146,6 @@ macro something(args...) return :($something($expr)) end -==(a::Some, b::Some)::Bool = a.value == b.value -# seperated out to ensure the above assertion works -==(::Some{Missing}, ::Some) = missing -==(::Some, ::Some{Missing}) = missing -==(::Some{Missing}, ::Some{Missing}) = missing - +==(a::Some, b::Some) = a.value == b.value isequal(a::Some, b::Some)::Bool = isequal(a.value, b.value) hash(s::Some, h::UInt) = hash(s.value, hash(Some, h)) From 5956ba4bf65d628ac5e71078b3909deffc7d979a Mon Sep 17 00:00:00 2001 From: Sukera <11753998+Seelengrab@users.noreply.github.com> Date: Thu, 28 Mar 2024 07:35:57 +0100 Subject: [PATCH 11/11] Apply suggestions from code review Co-authored-by: Lilith Orion Hafner --- base/some.jl | 3 ++- test/some.jl | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/base/some.jl b/base/some.jl index 3f95b04a45241..f0bad5cf59db4 100644 --- a/base/some.jl +++ b/base/some.jl @@ -148,4 +148,5 @@ end ==(a::Some, b::Some) = a.value == b.value isequal(a::Some, b::Some)::Bool = isequal(a.value, b.value) -hash(s::Some, h::UInt) = hash(s.value, hash(Some, h)) +const hash_some_seed = UInt == UInt64 ? 0xde5c997007a4ca3a : 0x78c29c09 +hash(s::Some, h::UInt) = hash(s.value, hash_some_seed + h) diff --git a/test/some.jl b/test/some.jl index 064eaed1d6ecb..59ccd05be96bf 100644 --- a/test/some.jl +++ b/test/some.jl @@ -73,6 +73,7 @@ # hashing implications @test hash(Some(0x1)) != hash(0x1) @test hash(Some(0x1)) == hash(Some(1)) +@test hash((Some(1),)) != hash((1, Some)) @testset "something" begin @test_throws ArgumentError something()