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

for loop invoked from generic procedure defined in another module cant find items iterator #11167

Open
SolitudeSF opened this issue May 3, 2019 · 11 comments

Comments

@SolitudeSF
Copy link
Contributor

SolitudeSF commented May 3, 2019

Template with for loop invoked from generic procedure defined in another cant find items iterator

Example

a.nim

iterator items(a: int): int = discard

template t(a: int): untyped =
  for _ in a:
    discard

proc p*(a: int | uint) =
  t a

test.nim

import a

p 1

Current Output

a.nim(1, 10) Hint: 'items' is declared but not used [XDeclaredButNotUsed]
test.nim(3, 3) template/generic instantiation of `p` from here
a.nim(8, 5) Error: type mismatch: got <int>
but expected one of: 
iterator items[T](a: seq[T]): T
iterator items(a: string): char
iterator items[IX, T](a: array[IX, T]): T
iterator items[T](a: set[T]): T
iterator items(a: cstring): char
iterator items[T](a: openArray[T]): T
iterator items[T](s: HSlice[T, T]): T
iterator items(E: typedesc[enum]): E:type

Expected Output

Should compile and run without errors.

Additional Information

Removing generic component from p or running p from the same module removes the error.

$ nim -v
Nim Compiler Version 0.19.9 [Linux: amd64]
Compiled at 2019-05-03
git hash: 515ab81477c1c3e4811c4fbf43a3ff81b87be970

links

@SolitudeSF SolitudeSF changed the title Template with for loop invoked from generic procedure defined in another cant find items iterator Template with for loop invoked from generic procedure defined in another module cant find items iterator May 3, 2019
@nc-x
Copy link
Contributor

nc-x commented May 5, 2019

It also works if items is exported or using for _ in a.items().

@krux02
Copy link
Contributor

krux02 commented May 5, 2019

I looks like semTemplate does not introduce the implicit items call for the iterator, and maybe it should not. If it should not, this is not a bug, but it is certainly something that should be documented.

@timotheecour
Copy link
Member

timotheecour commented Jul 16, 2019

looks like template is irrelevant, see reduced case below
@SolitudeSF maybe change the title to:

for loop invoked from generic procedure defined in another module cant find `items` iterator

?

reduced case: replace a.nim with:

  iterator items(a: int): int = discard
  proc p*(a: int | uint) =
    for _ in a:
      discard

@timotheecour
Copy link
Member

timotheecour commented Jul 16, 2019

workaround

use for _ in items(a) instead of for _ in a makes it work, both in original post as well as in reduced case i wrote above #11167 (comment)

(works both with or without mixin items)

potential solution

maybe semcheck of generic should early expand the for loop with items / pairs transformations, but needs to work with https://nim-lang.org/docs/manual.html#generics-symbol-lookup-in-generics in mind

EDIT

related (perhaps even duplicate) of #4773; see for example: #4773 (comment)
ideally though, instead of patching stdlib to use items/pairs everywhere, this should just work

@SolitudeSF SolitudeSF changed the title Template with for loop invoked from generic procedure defined in another module cant find items iterator for loop invoked from generic procedure defined in another module cant find items iterator Jul 17, 2019
@pmetras
Copy link

pmetras commented Jan 12, 2020

Another simple case where the problem occurs in nim 1.0.4 and #head:

First module ee.nim:

import sets

proc initH*[V]: HashSet[V] =
  result = initHashSet[V]()

proc foo*[V](h: var HashSet[V], c: seq[V]) =
  h = h + c.toHashSet()

Second module ff.nim

import hashes

import ee

type
  Choice = object
    i: int

proc hash(c: Choice): Hash =
  result = Hash(c.i)

var h = initH[Choice]()
let c = @[Choice(i: 1)]

foo(h, c)

Compilation of ff.nim shows the error:

$ nim c ff
Hint: used config file '/home/pierre/.choosenim/toolchains/nim-#devel/config/nim.cfg' [Conf]
Hint: used config file '/home/pierre/.choosenim/toolchains/nim-#devel/config/config.nims' [Conf]
Hint: system [Processing]
Hint: widestrs [Processing]
Hint: io [Processing]
Hint: ff [Processing]
Hint: hashes [Processing]
Hint: ee [Processing]
Hint: sets [Processing]
Hint: math [Processing]
Hint: bitops [Processing]
Hint: macros [Processing]
/home/pierre/alternaCloud/Temp/sdk/ff.nim(17, 4) template/generic instantiation of `foo` from here
/home/pierre/alternaCloud/Temp/sdk/ee.nim(9, 9) template/generic instantiation of `+` from here
/home/pierre/.choosenim/toolchains/nim-#devel/lib/pure/collections/sets.nim(487, 17) template/generic instantiation of `union` from here
/home/pierre/.choosenim/toolchains/nim-#devel/lib/pure/collections/sets.nim(411, 7) template/generic instantiation of `incl` from here
/home/pierre/.choosenim/toolchains/nim-#devel/lib/pure/collections/sets.nim(211, 15) Error: type mismatch: got <HashSet[ff.Choice]>
but expected one of: 
iterator items[T](a: seq[T]): T
  first type mismatch at position: 1
  required type for a: seq[T]
  but expression 'other' is of type: HashSet[ff.Choice]
iterator items[T](s: HSlice[T, T]): T
  first type mismatch at position: 1
  required type for s: HSlice[items.T, items.T]
  but expression 'other' is of type: HashSet[ff.Choice]
iterator items(a: string): char
  first type mismatch at position: 1
  required type for a: string
  but expression 'other' is of type: HashSet[ff.Choice]
iterator items[IX, T](a: array[IX, T]): T
  first type mismatch at position: 1
  required type for a: array[IX, T]
  but expression 'other' is of type: HashSet[ff.Choice]
iterator items[T](a: set[T]): T
  first type mismatch at position: 1
  required type for a: set[T]
  but expression 'other' is of type: HashSet[ff.Choice]
iterator items(a: cstring): char
  first type mismatch at position: 1
  required type for a: cstring
  but expression 'other' is of type: HashSet[ff.Choice]
iterator items[T](a: openArray[T]): T
  first type mismatch at position: 1
  required type for a: openArray[T]
  but expression 'other' is of type: HashSet[ff.Choice]
iterator items(E: typedesc[enum]): E:type
  first type mismatch at position: 1
  required type for E: type enum
  but expression 'other' is of type: HashSet[ff.Choice]

expression: items(other)

If both files are merged, the error disappear. Or if sets is imported in ff.nim and initH is not used.

@pmetras
Copy link

pmetras commented Jan 13, 2020

Simply adding import sets in the second module ff.nim makes the error disappear.

@timotheecour
Copy link
Member

bumping priority; keeps happening, see #16060

@oakes
Copy link

oakes commented Feb 21, 2021

I noticed that with the latest devel, sets.len also must be exported. I made foo.nim:

import sets, sequtils

proc dedupe*[T](arr: openArray[T]): seq[T] =
  arr.toHashSet.toSeq

export sets.items

...and bar.nim:

import foo

echo dedupe([1, 2, 3])

Running nim c -r bar.nim gives me:

C:\Users\sekao\Documents\Nim\lib\pure\collections\sets.nim(241, 17) Error: type mismatch: got <HashSet[system.int]>
but expected one of:
func len(x: (type array) | array): int
  first type mismatch at position: 1
  required type for x: typedesc[array] or array
  but expression 's' is of type: HashSet[system.int]
func len(x: string): int
func len[TOpenArray: openArray | varargs](x: TOpenArray): int                                                             first type mismatch at position: 1                                                                                      required type for x: TOpenArray: openArray or varargs                                                                   but expression 's' is of type: HashSet[system.int]
...

Exporting sets.len as well fixes it.

nim -v
Nim Compiler Version 1.5.1 [Windows: amd64]
Compiled at 2021-02-21
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 70ec17eede74f4216aee3b0bfb11e789bcbf0e54
active boot switches: -d:release

@timotheecour
Copy link
Member

@oakes you can send a PR to fix it similar to https://github.com/nim-lang/Nim/pull/16060/files

@timotheecour
Copy link
Member

timotheecour commented Mar 9, 2021

candidate places to look at for a fix:

  • implicitIterator
  • binding mechanism in generics that should transform for x in b by for x in items(b) (ditto with pairs) before binding is done; but possibly with mixing items ?

@Araq
Copy link
Member

Araq commented Mar 10, 2021

binding mechanism in generics that should transform for x in b by for x in items(b) (ditto with pairs) before binding is done; but possibly with mixing items ?

Yeah... But it changes the AST and isn't correct for when b ends up being an iterator...

Araq added a commit that referenced this issue Oct 25, 2024
closes nim-lang/RFCs#380, fixes #4773, fixes
#14729, fixes #16755, fixes #18150, fixes #22984, refs #11167 (only some
comments fixed), refs #12620 (needs tiny workaround)

The compiler gains a concept of root "nominal" types (i.e. objects,
enums, distincts, direct `Foo = ref object`s, generic versions of all of
these). Exported top-level routines in the same module as the nominal
types that their parameter types derive from (i.e. with
`var`/`sink`/`typedesc`/generic constraints) are considered attached to
the respective type, as the RFC states. This happens for every argument
regardless of placement.

When a call is overloaded and overload matching starts, for all
arguments in the call that already have a type, we add any operation
with the same name in the scope of the root nominal type of each
argument (if it exists) to the overload match. This also happens as
arguments gradually get typed after every overload match. This restricts
the considered overloads to ones attached to the given arguments, as
well as preventing `untyped` arguments from being forcefully typed due
to unrelated overloads. There are some caveats:

* If no overloads with a name are in scope, type bound ops are not
triggered, i.e. if `foo` is not declared, `foo(x)` will not consider a
type bound op for `x`.
* If overloads in scope do not have enough parameters up to the argument
which needs its type bound op considered, then type bound ops are also
not added. For example, if only `foo()` is in scope, `foo(x)` will not
consider a type bound op for `x`.

In the cases of "generic interfaces" like `hash`, `$`, `items` etc. this
is not really a problem since any code using it will have at least one
typed overload imported. For arbitrary versions of these though, as in
the test case for #12620, a workaround is to declare a temporary
"template" overload that never matches:

```nim
# neither have to be exported, just needed for any use of `foo`:
type Placeholder = object
proc foo(_: Placeholder) = discard
```

I don't know what a "proper" version of this could be, maybe something
to do with the new concepts.

Possible directions:

A limitation with the proposal is that parameters like `a: ref Foo` are
not attached to any type, even if `Foo` is nominal. Fixing this for just
`ptr`/`ref` would be a special case, parameters like `seq[Foo]` would
still not be attached to `Foo`. We could also skip any *structural* type
but this could produce more than one nominal type, i.e. `(Foo, Bar)`
(not that this is hard to implement, it just might be unexpected).

Converters do not use type bound ops, they still need to be in scope to
implicitly convert. But maybe they could also participate in the nominal
type consideration: if `Generic[T] = distinct T` has a converter to `T`,
both `Generic` and `T` can be considered as nominal roots.

The other restriction in the proposal, being in the same scope as the
nominal type, could maybe be worked around by explicitly attaching to
the type, i.e.: `proc foo(x: T) {.attach: T.}`, similar to class
extensions in newer OOP languages. The given type `T` needs to be
obtainable from the type of the given argument `x` however, i.e.
something like `proc foo(x: ref T) {.attach: T.}` doesn't work to fix
the `ref` issue since the compiler never obtains `T` from a given `ref
T` argument. Edit: Since the module is queried now, this is likely not
possible.

---------

Co-authored-by: Andreas Rumpf <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants