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

Error: ignored invalid for loop for it triggered when changing a # to a ## #9230

Open
timotheecour opened this issue Oct 6, 2018 · 4 comments

Comments

@timotheecour
Copy link
Member

timotheecour commented Oct 6, 2018

import tables

when defined(case1):
  # Error: ignored invalid for loop for it {.inject.} in s2:
  template items*[A,B](t: Table[A,B]): tuple[key: A, val: B] =
    ## foobar
    pairs(t)

when defined(case2):
  # ok
  template items*[A,B](t: Table[A,B]): tuple[key: A, val: B] =
    # foobar
    pairs(t)

when isMainModule:
  import sequtils
  let a = initTable[int, string]()
  let b = mapIt(a, it.key)

this prevents the simpler suggestion in #9217 (comment)

@skilchen
Copy link
Contributor

skilchen commented Oct 7, 2018

In your case1, if you start the doc comment on the template line the error goes away (i don't know why). runnableExamples will still trigger the error, but you could write something like:

when defined(case1):
  template items*[A,B](t: Table[A,B]): tuple[key: A, val: B] = ##
    ## iterates over (key, value) tuples in the table `t`.
    ##
    ## Example:
    ##
    ## .. code-block:: nim
    ##    import sequtils
    ##    let a = {10: "foo", 12: "bar"}.toTable
    ##    doAssert mapIt(a, it.key) == [10, 12]
    ##    doAssert mapIt(a, it.val)[1] == "bar"
    ##
    pairs(t)

You should come up with a better use case than this bizarre way of writing toSeq(keys(a)) and toSeq(values(a))[1]. In your PR #9217 please give an example in which an items iterator on tables would actually be useful.

@Araq
Copy link
Member

Araq commented Oct 9, 2018

Please, please stop writing these when defined(case1): they are so messy. What is "case1"? Does it fail? Does it work? Should it compile and doesn't or does it compile and shouldn't?

@pmetras
Copy link

pmetras commented May 25, 2020

The reason of the bug is that the test here considers that iterator is not used in a for loop because a doc comment adds a statement in the parsed tree, contrarily to a simple comment.

Another example of such bug where using a template is required by the context:

type
  Melon = object

template pairs*(m: Melon): tuple[key: int, val: int] =
  let iter = iterator(melon: Melon): tuple[key: int, val: int] =
    for i in a .. 10:
      yield (key: i, val: i * i)
  iter(m)

block:
  let a = 2
  let m = Melon()
  for k, v in m:
    echo "k=", k, "; v=", v

@timotheecour
Copy link
Member Author

this crops up in many places, eg #14420 (comment)

when true: # D20200525T015650
  type Foo = object
    x: float

  proc fn(a: var Foo): var float =
    ## discard <- turn this into a comment (or a `discard`) and error disappears
    # result = a.x # this would work
    a.x #  Error: limited VM support for 'addr'

  template main =
    var a = Foo()
    discard fn(a)
  main() # works
  static: main()  # fails

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

No branches or pull requests

4 participants