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

Table has inconsistent hash #20023

Closed
rotu opened this issue Jul 14, 2022 · 3 comments · Fixed by #20346
Closed

Table has inconsistent hash #20023

rotu opened this issue Jul 14, 2022 · 3 comments · Fixed by #20346

Comments

@rotu
Copy link
Contributor

rotu commented Jul 14, 2022

The hash of a Table depends on how that Table was derived.

This violates the prime rule of hashes (If two values compare equal, their hashes must also be equal) and results in corrupt data structures when Table is used as a key in a hashed container.

Example

import std/tables
import std/hashes

let t = ()
var a = toTable({t:t})
del(a,t)
let b = default(typeof(a))

assert a==b , "tables are equal"
# this following assertion fails!
assert hash(a) == hash(b), "table hashes are equal"

Current Output

Error: unhandled exception: /Users/dan/Documents/fates/nim/fates/issue.nim(11, 8) `hash(a) == hash(b)` table hashes are equal [AssertionDefect]

Expected Output

no error

Possible Solution

  • I believe the issue is that the hash depends on the allocated table size, since this also fails:
var a=initTable[int,int](initialSize=10)
var b=initTable[int,int](initialSize=100)
assert a==b , "tables are equal"
# this following assertion fails!
assert hash(a) == hash(b), "table hashes are equal"

Additional Information

$ nim -v
Nim Compiler Version 1.6.6 [MacOSX: arm64]
Compiled at 2022-05-05
Copyright (c) 2006-2021 by Andreas Rumpf

active boot switches: -d:release -d:nimUseLinenoise
@rotu
Copy link
Contributor Author

rotu commented Jul 14, 2022

Workaround below. I'm not sure that this is a good idea (e.g. multiple parts of code may have conflicting hashes for the same object). I think this needs to be part of std/tables.

import std/[tables, hashes]

proc hash*[K,V](s: Table[K,V]): Hash =
  for p in pairs(s):
    result = result xor hash(p)
  result = !$result

@planetis-m
Copy link
Contributor

Is there even a hash for Table? https://nim-lang.github.io/Nim/theindex.html#hash I think it picks the generic hash for objects.

@rotu
Copy link
Contributor Author

rotu commented Jul 14, 2022

@planetis-m Yes, that does seem to be the case, which bit me quite badly.

It would be nice if defining a new equality operator somehow undefined the hash function, so this would be caught at compile time.

Oddly enough, there’s an order-invariant hash for ordered tables of JSON nodes (rather than correctly defining the hash on the JsonNodeObj class)

proc hash*(n: OrderedTable[string, JsonNode]): Hash =

bung87 added a commit to bung87/Nim that referenced this issue Sep 13, 2022
Araq added a commit that referenced this issue Jun 21, 2023
* fix #20023 hash for generic tables

* use default computation

* Update lib/pure/collections/tables.nim

Co-authored-by: Dan Rose <dan@digilabs.io>

* Update lib/pure/collections/tables.nim

Co-authored-by: Dan Rose <dan@digilabs.io>

* Update lib/pure/collections/tables.nim

* Update lib/pure/collections/tables.nim

* Update t20023.nim

---------

Co-authored-by: Dan Rose <dan@digilabs.io>
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
bung87 added a commit to bung87/Nim that referenced this issue Jul 29, 2023
* fix nim-lang#20023 hash for generic tables

* use default computation

* Update lib/pure/collections/tables.nim

Co-authored-by: Dan Rose <dan@digilabs.io>

* Update lib/pure/collections/tables.nim

Co-authored-by: Dan Rose <dan@digilabs.io>

* Update lib/pure/collections/tables.nim

* Update lib/pure/collections/tables.nim

* Update t20023.nim

---------

Co-authored-by: Dan Rose <dan@digilabs.io>
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
narimiran pushed a commit that referenced this issue Sep 11, 2023
* fix #20023 hash for generic tables

* use default computation

* Update lib/pure/collections/tables.nim

Co-authored-by: Dan Rose <dan@digilabs.io>

* Update lib/pure/collections/tables.nim

Co-authored-by: Dan Rose <dan@digilabs.io>

* Update lib/pure/collections/tables.nim

* Update lib/pure/collections/tables.nim

* Update t20023.nim

---------

Co-authored-by: Dan Rose <dan@digilabs.io>
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
(cherry picked from commit 3ad2e7d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants