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

HashSet misses elements when excl'ing while iterating over it #15767

Closed
liquidev opened this issue Oct 28, 2020 · 1 comment
Closed

HashSet misses elements when excl'ing while iterating over it #15767

liquidev opened this issue Oct 28, 2020 · 1 comment

Comments

@liquidev
Copy link
Contributor

Example

import std/sets
import std/hashes

type
  Vec2i = tuple[x, y: int32]

proc hash(vec: Vec2i): Hash =
  result = Hash(0)
  result = result !& hash(vec.x)
  result = result !& hash(vec.y)
  result = !$result

var set: HashSet[Vec2i]

for y in 1..4:
  for x in 0..<32:
    let vec: Vec2i = (x.int32, y.int32)
    set.incl(vec)

let i = set.len

var j = 0
for vec in set:
  inc(j)
  set.excl(vec)
echo (i, j)

Current Output

(128, 96)

32 elements are magically gone from the set while we're iterating over it.

Expected Output

(128, 128)

Additional Information

If we change the HashSet to an OrderedSet, the program gets stuck at an infinite loop. Adding echo vec to the loop at the end of the program yields that it gets stuck at (x: 22, y: 1). ^C'ing out of the program, I could trace that it gets stuck in this while loop:

template forAllOrderedPairs(yieldStmt: untyped) {.dirty.} =
if s.data.len > 0:
var h = s.first
var idx = 0
while h >= 0:
var nxt = s.data[h].next
if isFilled(s.data[h].hcode):
yieldStmt
inc(idx)
h = nxt

If sets cannot handle excluding elements while iterating over them, then that should be clearly stated in the documentation and ideally also trigger an assertion.

$ nim -v
Nim Compiler Version 1.5.1 [Linux: amd64]
Compiled at 2020-10-22
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: 2cb484cefb13e4663afd8e0eb2ad81d43f097803
active boot switches: -d:release
@cooldome
Copy link
Member

Yes, it is not safe to iterate over container and modify it at the same time. Nim should raise an error in debug build.

ringabout added a commit to ringabout/Nim that referenced this issue Feb 7, 2021
@Araq Araq closed this as completed in 0cf3ba1 Feb 8, 2021
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
* fix some warnings

* close nim-lang#15767

* Revert "fix some warnings"

This reverts commit 39f2f23.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants