usage of `[]` on seqs within a generic wrongly prioritizes user-defined converter + overload over `system.[]`

[Quelklef]

If there is an overload for [] with the type [T](s: seq[T], n: int): T, then trying to .incl on a HashSet will fail.

Example

import sets

proc `[]`[T](s: seq[T], n: int): T =
  discard

var myset = initHashSet[int]()
myset.incl(1)

Current Output

/home/runner/bugxnim/main.nim(7, 6) template/generic instantiation of `incl` from here
/usr/local/lib/nim/pure/collections/setimpl.nim(52, 14) template/generic instantiation of `enlarge` from here
/usr/local/lib/nim/pure/collections/setimpl.nim(43, 16) template/generic instantiation of `rawInsert` from here
/usr/local/lib/nim/pure/collections/setimpl.nim(29, 10) Error: 'data[h].key' cannot be assigned to

Expected Output

No error

Possible Solution

Unclear to me

Additional Information

If you change the overload for [] to return a var T instead of a T, then there is no error. I assume, then, that the issue is that setimpl.nim(29, 10) is, for some reason, using my [] overload instead of the default.

It is unclear to me why this is happening, though I will admit it's been a few years since I've used Nim. Regardless, I find it highly unlikely that this is expected behaviour, especially since incl is a proc and not a template. Let me know if I am wrong.

[Araq]

It uses your [] overload because the language spec says so, see the section about closed vs open symbols. Now while I agree that's a bad idea for your case, the real issue here is that you introduced your own [] accesor for seqs, really bad code.

[Quelklef]

The actual overload I was using was [T](s: seq[T], n: BigInt): T, which seems perfectly reasonable to me.

Thanks for pointing me to open / closed symbols; sorry for opening an issue for expected behaviour.

[Quelklef]

Oh, the issue stemmed from the fact that I also had a converter from int to BigInt.

However, this seems to contradict the Nim manual, which states that an implicit conversion will only be used in overloading resolution if the arguments cannot match without conversion.

If I am not mistaken again, I'd be willing to look more into this.

[Araq]

If I am not mistaken again, I'd be willing to look more into this.

Fine with me but the reason will most likely be that system.[] for seqs was under-specified as we didn't anticipate custom overloads.

[Quelklef]

Here's my MWE

type Good = object
type Bad = object
type CustomKey = object

# vv Must be generic, of course
proc first[T](xs: seq[T]): T = xs[0]

converter intToKey(x: int): CustomKey = discard
proc `[]`(s: seq[Good], k: CustomKey): Bad = discard
# ^^ May be concrete or may be generic with `s: seq[T]`

var xs: seq[Good]
first(xs)

The expression xs[0] uses my converter + overload, causing the type system to complain that xs[0]: Bad is not a T = Good.

Removing the converter or the [] overload removes the error. (Though it still won't compile for other reasons).

I'm assuming that the relevant system.[] overload is proc `[]`[I: Ordinal; T](a: T; i: I): T? Even though this is very generic, should it still not take priority over a converter, as per the previously linked spec?

[Quelklef]

Also, a top-level reference to xs[0] will work properly:

var g: Good
g = xs[0]  # ok

[timotheecour]

related: #15911