Skip to content

Commit

Permalink
remove conflicting default call in tables.getOrDefault (#24265)
Browse files Browse the repository at this point in the history
fixes #23587

As explained in the issue, `getOrDefault` has a parameter named
`default` that can be a proc after generic instantiation. But the
parameter having a proc type [overrides all other
overloads](https://github.com/nim-lang/Nim/blob/f73e03b1323cf3ed65d6996f43c8e6891c40f924/compiler/semexprs.nim#L1203)
including the magic `system.default` overload and causes a compile error
if the proc doesn't match the normal use of `default`. To fix this, the
`result = default(B)` initializer call is removed because it's not
needed, `result` is always set in `getOrDefaultImpl` when a default
value is provided.

This is still a suspicious behavior of the compiler but `tables` working
has a higher priority.
  • Loading branch information
metagn authored Oct 9, 2024
1 parent 95a7695 commit 67ea754
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 25 deletions.
2 changes: 1 addition & 1 deletion compiler/docgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ proc add(dest: var ItemPre, str: string) = dest.add ItemFragment(isRst: false, s

proc addRstFileIndex(d: PDoc, fileIndex: lineinfos.FileIndex): rstast.FileIndex =
let invalid = rstast.FileIndex(-1)
result = d.nimToRstFid.getOrDefault(fileIndex, default = invalid)
result = d.nimToRstFid.getOrDefault(fileIndex, invalid)
if result == invalid:
let fname = toFullPath(d.conf, fileIndex)
result = addFilename(d.sharedState, fname)
Expand Down
2 changes: 1 addition & 1 deletion lib/packages/docutils/rst.nim
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ proc len(filenames: RstFileTable): int = filenames.idxToFilename.len
proc addFilename*(s: PRstSharedState, file1: string): FileIndex =
## Returns index of filename, adding it if it has not been used before
let nextIdx = s.filenames.len.FileIndex
result = getOrDefault(s.filenames.filenameToIdx, file1, default = nextIdx)
result = getOrDefault(s.filenames.filenameToIdx, file1, nextIdx)
if result == nextIdx:
s.filenames.filenameToIdx[file1] = result
s.filenames.idxToFilename.add file1
Expand Down
42 changes: 20 additions & 22 deletions lib/pure/collections/tables.nim
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,9 @@ proc getOrDefault*[A, B](t: Table[A, B], key: A): B =
result = default(B)
getOrDefaultImpl(t, key)

proc getOrDefault*[A, B](t: Table[A, B], key: A, default: B): B =
proc getOrDefault*[A, B](t: Table[A, B], key: A, def: B): B =
## Retrieves the value at `t[key]` if `key` is in `t`.
## Otherwise, `default` is returned.
## Otherwise, `def` is returned.
##
## See also:
## * `[] proc<#[],Table[A,B],A>`_ for retrieving a value of a key
Expand All @@ -434,8 +434,7 @@ proc getOrDefault*[A, B](t: Table[A, B], key: A, default: B): B =
let a = {'a': 5, 'b': 9}.toTable
doAssert a.getOrDefault('a', 99) == 5
doAssert a.getOrDefault('z', 99) == 99
result = default(B)
getOrDefaultImpl(t, key, default)
getOrDefaultImpl(t, key, def)

proc mgetOrPut*[A, B](t: var Table[A, B], key: A, val: B): var B =
## Retrieves value at `t[key]` or puts `val` if not present, either way
Expand Down Expand Up @@ -972,9 +971,9 @@ proc getOrDefault*[A, B](t: TableRef[A, B], key: A): B =

getOrDefault(t[], key)

proc getOrDefault*[A, B](t: TableRef[A, B], key: A, default: B): B =
proc getOrDefault*[A, B](t: TableRef[A, B], key: A, def: B): B =
## Retrieves the value at `t[key]` if `key` is in `t`.
## Otherwise, `default` is returned.
## Otherwise, `def` is returned.
##
## See also:
## * `[] proc<#[],TableRef[A,B],A>`_ for retrieving a value of a key
Expand All @@ -988,7 +987,7 @@ proc getOrDefault*[A, B](t: TableRef[A, B], key: A, default: B): B =
doAssert a.getOrDefault('a', 99) == 5
doAssert a.getOrDefault('z', 99) == 99

getOrDefault(t[], key, default)
getOrDefault(t[], key, def)

proc mgetOrPut*[A, B](t: TableRef[A, B], key: A, val: B): var B =
## Retrieves value at `t[key]` or puts `val` if not present, either way
Expand Down Expand Up @@ -1491,9 +1490,9 @@ proc getOrDefault*[A, B](t: OrderedTable[A, B], key: A): B =
result = default(B)
getOrDefaultImpl(t, key)

proc getOrDefault*[A, B](t: OrderedTable[A, B], key: A, default: B): B =
proc getOrDefault*[A, B](t: OrderedTable[A, B], key: A, def: B): B =
## Retrieves the value at `t[key]` if `key` is in `t`.
## Otherwise, `default` is returned.
## Otherwise, `def` is returned.
##
## See also:
## * `[] proc<#[],OrderedTable[A,B],A>`_ for retrieving a value of a key
Expand All @@ -1506,8 +1505,7 @@ proc getOrDefault*[A, B](t: OrderedTable[A, B], key: A, default: B): B =
let a = {'a': 5, 'b': 9}.toOrderedTable
doAssert a.getOrDefault('a', 99) == 5
doAssert a.getOrDefault('z', 99) == 99
result = default(B)
getOrDefaultImpl(t, key, default)
getOrDefaultImpl(t, key, def)

proc mgetOrPut*[A, B](t: var OrderedTable[A, B], key: A, val: B): var B =
## Retrieves value at `t[key]` or puts `val` if not present, either way
Expand Down Expand Up @@ -1992,9 +1990,9 @@ proc getOrDefault*[A, B](t: OrderedTableRef[A, B], key: A): B =

getOrDefault(t[], key)

proc getOrDefault*[A, B](t: OrderedTableRef[A, B], key: A, default: B): B =
proc getOrDefault*[A, B](t: OrderedTableRef[A, B], key: A, def: B): B =
## Retrieves the value at `t[key]` if `key` is in `t`.
## Otherwise, `default` is returned.
## Otherwise, `def` is returned.
##
## See also:
## * `[] proc<#[],OrderedTableRef[A,B],A>`_ for retrieving a value of a key
Expand All @@ -2008,7 +2006,7 @@ proc getOrDefault*[A, B](t: OrderedTableRef[A, B], key: A, default: B): B =
doAssert a.getOrDefault('a', 99) == 5
doAssert a.getOrDefault('z', 99) == 99

getOrDefault(t[], key, default)
getOrDefault(t[], key, def)

proc mgetOrPut*[A, B](t: OrderedTableRef[A, B], key: A, val: B): var B =
## Retrieves value at `t[key]` or puts `val` if not present, either way
Expand Down Expand Up @@ -2320,9 +2318,9 @@ proc rawGet[A](t: CountTable[A], key: A): int =
h = nextTry(h, high(t.data))
result = -1 - h # < 0 => MISSING; insert idx = -1 - result

template ctget(t, key, default: untyped): untyped =
template ctget(t, key, def: untyped): untyped =
var index = rawGet(t, key)
result = if index >= 0: t.data[index].val else: default
result = if index >= 0: t.data[index].val else: def

proc inc*[A](t: var CountTable[A], key: A, val = 1)

Expand Down Expand Up @@ -2447,15 +2445,15 @@ proc contains*[A](t: CountTable[A], key: A): bool =
## the `in` operator.
return hasKey[A](t, key)

proc getOrDefault*[A](t: CountTable[A], key: A; default: int = 0): int =
proc getOrDefault*[A](t: CountTable[A], key: A; def: int = 0): int =
## Retrieves the value at `t[key]` if `key` is in `t`. Otherwise, the
## integer value of `default` is returned.
## integer value of `def` is returned.
##
## See also:
## * `[] proc<#[],CountTable[A],A>`_ for retrieving a value of a key
## * `hasKey proc<#hasKey,CountTable[A],A>`_ for checking if a key
## is in the table
ctget(t, key, default)
ctget(t, key, def)

proc del*[A](t: var CountTable[A], key: A) {.since: (1, 1).} =
## Deletes `key` from table `t`. Does nothing if the key does not exist.
Expand Down Expand Up @@ -2772,15 +2770,15 @@ proc contains*[A](t: CountTableRef[A], key: A): bool =
## the `in` operator.
return hasKey[A](t, key)

proc getOrDefault*[A](t: CountTableRef[A], key: A, default: int): int =
proc getOrDefault*[A](t: CountTableRef[A], key: A, def: int): int =
## Retrieves the value at `t[key]` if `key` is in `t`. Otherwise, the
## integer value of `default` is returned.
## integer value of `def` is returned.
##
## See also:
## * `[] proc<#[],CountTableRef[A],A>`_ for retrieving a value of a key
## * `hasKey proc<#hasKey,CountTableRef[A],A>`_ for checking if a key
## is in the table
result = t[].getOrDefault(key, default)
result = t[].getOrDefault(key, def)

proc len*[A](t: CountTableRef[A]): int =
## Returns the number of keys in `t`.
Expand Down
2 changes: 1 addition & 1 deletion testament/important_packages.nim
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ pkg "polypbren"
pkg "presto"
pkg "prologue", "nimble tcompile"
# remove fork after https://github.com/PMunch/combparser/pull/7 is merged:
pkg "protobuf", "nimble install -y https://github.com/metagn/combparser@#HEAD; nim c -o:protobuff -r src/protobuf.nim"
pkg "protobuf", "nimble uninstall -i -y combparser; nimble install -y https://github.com/metagn/combparser@#HEAD; nimble install --depsOnly -y; nim c -o:protobuff -r src/protobuf.nim"
pkg "rbtree"
pkg "react", "nimble example"
pkg "regex", "nim c src/regex"
Expand Down
15 changes: 15 additions & 0 deletions tests/stdlib/ttables.nim
Original file line number Diff line number Diff line change
Expand Up @@ -314,3 +314,18 @@ block countTableWithoutInit:
d.inc("f")
merge(d, e)
doAssert d["f"] == 7

block: # issue #23587
type
A = proc ()

proc main =
let repo = initTable[uint32, A]()

let c1 = repo.getOrDefault(uint32(1), nil)
doAssert c1.isNil

let c2 = repo.getOrDefault(uint32(1), A(nil))
doAssert c2.isNil

main()

0 comments on commit 67ea754

Please sign in to comment.