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

toCritBitTree funcs have wrong signature #16554

Closed
konsumlamm opened this issue Jan 2, 2021 · 0 comments · Fixed by #16564
Closed

toCritBitTree funcs have wrong signature #16554

konsumlamm opened this issue Jan 2, 2021 · 0 comments · Fixed by #16564

Comments

@konsumlamm
Copy link
Contributor

The toCritBitTree func (from the critbits module) doesn't work with non-string values.

Example

import critbits

echo {"a": 0, "b": 1}.toCritBitTree

Current Output

Error:

/home/<censored>/Projects/Nim/testing/src/testing.nim(17, 25) template/generic instantiation of `toCritBitTree` from here
/home/<censored>/.choosenim/toolchains/nim-#devel/lib/pure/collections/critbits.nim(538, 28) Error: type mismatch: got <CritBitTree[system.string], string, int>
but expected one of: 
proc incl[T](c: var CritBitTree[T]; key: string; val: T)
  first type mismatch at position: 3
  required type for val: T
  but expression 'item[1]' is of type: int
3 other mismatching symbols have been suppressed; compile with --showAllMismatches:on to see them

expression: incl(result, item[0], item[1])

Expected Output

{"a": 0, "b": 1}

Possible Solution

Currently, the signatures of the toCritBitTree funcs are as follows (https://github.com/nim-lang/Nim/blob/devel/lib/pure/collections/critbits.nim#L534):

func toCritBitTree*[A, B](pairs: openArray[(A, B)]): CritBitTree[A]
func toCritBitTree*[T](items: openArray[T]): CritBitTree[void]

If the result type for the first version is changed to CritBitTree[B], the bug is fixed (since A is the key type and B the value type). However, the generic type parameter for the keys (A and T) is unnecessary in both versions, since CritBitTrees always have string keys. So the fixed signatures should be (the actual implementation doesn't need to change):

func toCritBitTree*[T](pairs: openArray[(string, T)]): CritBitTree[T]
func toCritBitTree*(items: openArray[string]): CritBitTree[void]

This both fixes the bug and produces better error messages when someone attempts to use non-string keys.

Additional Information

This is not a regression, since the code didn't change since it's addition in #15444.

$ nim -v
Nim Compiler Version 1.5.1
git hash: 297c8e403d110dd872e070563328f4e0c734cd01
@metagn metagn mentioned this issue Jan 3, 2021
Araq pushed a commit that referenced this issue Jan 3, 2021
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
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.

1 participant