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

SortedSet returns different results for equal indexes. #138

Open
1 of 2 tasks
Moriquendi opened this issue Feb 2, 2022 · 3 comments
Open
1 of 2 tasks

SortedSet returns different results for equal indexes. #138

Moriquendi opened this issue Feb 2, 2022 · 3 comments
Labels
bug Something isn't working
Milestone

Comments

@Moriquendi
Copy link

I encountered the issue when trying to get an absolute position of the element and got incorrect results.
I started a conversation on forum because I wasn't sure if it's a bug.

let index = set.index(of: X)
print(index == set.startIndex) // true
print(set[index] == set[set.startIndex]) // false

Information

Package version: Branch feature/SortedCollections
Platform version: macOS 12.2 Beta (21D5025f)
Swift version:

swift-driver version: 1.26.21 Apple Swift version 5.5.2 (swiftlang-1300.0.47.5 clang-1300.0.29.30)
Target: arm64-apple-macosx12.0

Checklist

  • If possible, I've reproduced the issue using the main branch of this package.
  • I've searched for existing GitHub issues.

Steps to Reproduce

Sample code:

var set = SortedSet<Int>()
        
for i in 0..<50 {
   set.insert(i)
}
        
set.forEach {
   let i = set.index(of: $0)!
   let v = set[i]
   let d = set.distance(from: set.startIndex, to: i)
   print("\(v) --> \(d)")

   if i == set.startIndex {
      print("Current `index` equals `startIndex`.")
      print("set[index] = \(set[i]). set[startIndex] = \(set[set.startIndex])")
   }
}

Output:

0 --> 0
Current index equals startIndex.
set[index] = 0. set[startIndex] = 0
1 --> 1
2 --> 0
Current index equals startIndex.
set[index] = 2. set[startIndex] = 0
3 --> 3
4 --> 4
5 --> 1
6 --> 6
7 --> 7
8 --> 0
...

Expected behavior

When indexes are equal, getting a value for those indexes should return the same value.

Actual behavior

Even though indexes are equal, getting a value for them returns different results.

@Moriquendi Moriquendi added the bug Something isn't working label Feb 2, 2022
@lorentey
Copy link
Member

lorentey commented Feb 2, 2022

Yep, this is definitely a bug!

(Beware, SortedSet isn't ready for production use yet. These reports are very much appreciated, though!)

@Moriquendi
Copy link
Author

(Beware, SortedSet isn't ready for production use yet. These reports are very much appreciated, though!)

Yup, I saw the Readme notice but I needed a SortedSet in my app and thought I'll give Swift Collections a try :)
In the future, if time allows, I'll try to contribute fixes on my own. But in the meantime, I hope these bug reports will be helpful :)

@lorentey lorentey added this to the 1.1.0 milestone Sep 19, 2022
@lorentey lorentey modified the milestones: 1.1.0, 1.2.0 Oct 14, 2022
@SaudKadiri
Copy link

SaudKadiri commented May 23, 2023

After spending some time I figured out:

  1. The bug is across the SortedCollections module i.e in SortedSet, SortedDictionary and _BTree(root cause of the bug lies in here).
  • Observed in SortedSet
func test_indexOf() {
  var set = SortedSet<Int>()
      
  for i in 0..<50 {
    set.insert(i)
  }
  for (item, index) in zip(set, set.indices) {
      let i = set.index(of: item)!
      if i != index {
        print("Some issue with index(of:)")
        break
      }
  }
}

Output:

Some issue with index(of:)
  • Observed in SortedDictionary
func test_indexForKey() {
  var sortedDict: SortedDictionary<Int, String> = [:]
  for i in 0..<50 {
    sortedDict[i] = "\(i)"
  }
  for (index, item) in zip(sortedDict.indices, sortedDict) {
    let i = sortedDict.index(forKey: item.key)!
    if i != index {
      print("Some issue with index(forKey:)")
      break
    } 
  }
}

Output:

Some issue with index(forKey:)
  1. SortedSet.index(of:) and SortedDictionary.index(forKey:) are basically a call to _BTrees's findAnyIndex(forKey key: Key) -> Index? function. So let's test if _BTree.findAnyIndex(forKey:) is correct. I used this test case.
func test_findAnyIndex() {
  var tree = _BTree<Int, String>()
  for (key, value) in (0..<50).map({ ($0, "\($0)") }) {
    tree.updateAnyValue(value, forKey: key)
  }
  for (index, item) in zip(tree.indices, tree) { 
    let i = tree.findAnyIndex(forKey: item.key)
    if i != index {
      print("Problems with findAnyIndex")
      break
    }
  }
}

Output:

Problems with findAnyIndex
  1. I then wrote my own findAnyIndex as such (O(log n) implementation):
extension _BTree {
  @inlinable
  internal func findAnyIndex(forKey key: Key) -> Index? {
    var (lo, hi) = (startIndex, endIndex)
    while lo < hi {
        let mid = index(lo, offsetBy: distance(from: lo, to: hi)/2)
        if self[mid].key < key {
            lo = index(after: mid)
        } else {
            hi = mid
        }
    }
    return lo < endIndex && self[lo].key == key ? lo : nil
  }
}

and all the bugs vanished.
The test case in the issue gives the following output now.

0 --> 0
Current `index` equals `startIndex`.
set[index] = 0. set[startIndex] = 0
1 --> 1
2 --> 2
3 --> 3
4 --> 4
5 --> 5
6 --> 6
7 --> 7
8 --> 8
9 --> 9
10 --> 10
11 --> 11
12 --> 12
13 --> 13
14 --> 14
15 --> 15
16 --> 16
17 --> 17
18 --> 18
19 --> 19
20 --> 20
21 --> 21
22 --> 22
23 --> 23
24 --> 24
25 --> 25
26 --> 26
27 --> 27
28 --> 28
29 --> 29
30 --> 30
31 --> 31
32 --> 32
33 --> 33
34 --> 34
35 --> 35
36 --> 36
37 --> 37
38 --> 38
39 --> 39
40 --> 40
41 --> 41
42 --> 42
43 --> 43
44 --> 44
45 --> 45
46 --> 46
47 --> 47
48 --> 48
49 --> 49

And also the test cases I made are all working just fine now.

Hope this helps!🤝

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants