Turn hash for GapObj into error (#891)

... instead of providing a trivial (constant) hash. This may break
some trivial applications, but in return it helps us identify code
that relies on the useless default hash, i.e., it helps us find
performance bottlenecks.

This is a breaking change so also increase the package version.

CHANGES.md

@@ -1,5 +1,13 @@
 # Changes in GAP.jl
 
+## Version 0.10.0-DEV (released YYYY-MM-DD)
+
+- Changed `hash(::GapObj)` to throw an error (no general non-trivial hashing is
+  possible for general GAP objects)
+- Remove support for conversion to `AbstractString` (it was not meaningful anyway,
+  and hopefully nobody used it; but if you did, just convert to `String` instead
+  for an identical outcome)
+
 ## Version 0.9.8 (released 2023-09-11)
 
 - Allow GAP.Obj(x,true) for recursive conversion (#910. #925)

Project.toml

@@ -1,7 +1,7 @@
 name = "GAP"
 uuid = "c863536a-3901-11e9-33e7-d5cd0df7b904"
 authors = ["Thomas Breuer <[email protected]>", "Sebastian Gutsche <[email protected]>", "Max Horn <[email protected]>"]
-version = "0.9.8"
+version = "0.10.0-DEV"
 
 [deps]
 Artifacts = "56f22d72-fd6d-98f1-02f0-08ddc0907c33"

src/adapter.jl

@@ -361,11 +361,12 @@ for (left, right) in typecombinations
 end
 
 # Since we define the equality of GAP objects (see above),
-# we must provide a safe `hash` method, otherwise `==` results may be wrong.
+# we must provide a safe `hash` method, otherwise using GAP objects
+# in dictionaries or `Set`s can lead to inconsistent results.
 # For example, `==` for `Set`s of GAP objects may erroneously return
 # `false` if the default `hash` is used.
-Base.hash(::GapObj, h::UInt) = h
-Base.hash(::FFE, h::UInt) = h
+Base.hash(::GapObj, h::UInt) = error("hash for GAP objects is not implemented")
+Base.hash(::FFE, h::UInt) = error("hash for GAP objects is not implemented")
 
 ### RNGs
 

src/constructors.jl

@@ -389,23 +389,6 @@ Set{Any} with 2 elements:
   Any[1]
   Any[2]
 ```
-
-In the following examples,
-the order in which the Julia output is shown may vary.
-
-# Examples
-```jldoctest
-julia> s = Set{Any}(GAP.Obj([[1], [2], [1]]; recursive=true); recursive=false);
-
-julia> s == Set{Any}([GAP.Obj([1]), GAP.Obj([2])])
-true
-
-julia> s = Set{Any}(GAP.Globals.SymmetricGroup(2); recursive=false);
-
-julia> s == Set{Any}([GAP.evalstr("()"), GAP.evalstr("(1,2)")])
-true
-
-```
 """
 Base.Set{T}(obj::GapObj; recursive::Bool = true) where {T} =
     gap_to_julia(Set{T}, obj; recursive)

test/basics.jl

@@ -28,21 +28,13 @@
     y = GAP.evalstr("[]")
     @test !(x === y)
     @test (x == y)
-    @test hash(x) == 0
+    @test_throws ErrorException hash(x)
 
     x = GAP.evalstr("Z(2)")
     y = GAP.evalstr("Z(4)^3")
     @test !(x === y)
     @test (x == y)
-    @test hash(x) == 0
-
-    # The following would sometimes fail if no dedicated `hash`
-    # method would be available for GAP objects.
-    for i in 1:100
-        x = Set{Any}([GAP.evalstr("()"), GAP.evalstr("(1,2)")])
-        y = Set{Any}([GAP.evalstr("()"), GAP.evalstr("(1,2)")])
-        @test (x == y)
-    end
+    @test_throws ErrorException hash(x)
 end
 
 @testset "globals" begin

test/constructors.jl

@@ -149,7 +149,7 @@
     @test (@inferred Set{Vector{Int}}(GAP.evalstr("[[1,2],[2,3,4]]"))) == Set([[1, 2], [2, 3, 4]])
     @test (@inferred Set{String}(GAP.evalstr("[\"b\", \"a\", \"b\"]"))) == Set(["b", "a", "b"])
     x = GAP.evalstr("SymmetricGroup(3)")
-    @test (@inferred Set{GAP.GapObj}(x)) == Set{GAP.GapObj}(GAP.Globals.AsSet(x))
+    #@test (@inferred Set{GAP.GapObj}(x)) == Set{GAP.GapObj}(GAP.Globals.AsSet(x))
   end
 
   @testset "Tuples" begin

test/conversion.jl

@@ -173,12 +173,12 @@
     x = GAP.evalstr("[ [ 1 ], [ 2 ], [ 1 ] ]")
     y = [GAP.evalstr("[ 1 ]"), GAP.evalstr("[ 2 ]")]
     @test (@inferred GAP.gap_to_julia(Set{Vector{Int}}, x)) == Set([[1], [2], [1]])
-    @test @inferred GAP.gap_to_julia(Set{GAP.GapObj}, x, recursive = false) == Set(y)
-    @test @inferred GAP.gap_to_julia(Set{Any}, x, recursive = false) == Set(y)
+    #@test @inferred GAP.gap_to_julia(Set{GAP.GapObj}, x, recursive = false) == Set(y)
+    #@test @inferred GAP.gap_to_julia(Set{Any}, x, recursive = false) == Set(y)
     @test (@inferred GAP.gap_to_julia(Set{Any}, x)) == Set([[1], [2], [1]])
     x = GAP.evalstr("[ Z(2), Z(3) ]")  # a non-collection
     y = [GAP.evalstr("Z(2)"), GAP.evalstr("Z(3)")]
-    @test GAP.gap_to_julia(Set{GAP.FFE}, x) == Set(y)
+    #@test GAP.gap_to_julia(Set{GAP.FFE}, x) == Set(y)
   end
 
   @testset "Tuples" begin