Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn hash for GapObj into error #891

Merged
merged 1 commit into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
7 changes: 4 additions & 3 deletions src/adapter.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 0 additions & 17 deletions src/constructors.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 2 additions & 10 deletions test/basics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/constructors.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions test/conversion.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading