nim-lang / RFCs

A repository for your Nim proposals.
136 stars 23 forks source link

remove all 50 `pairs` overloads in favor of `macro pairs` based on ForLoopStmt #74

Open timotheecour opened 5 years ago

timotheecour commented 5 years ago

/cc @Araq

stdlib defines 50 overloads of pairs ; a lot of these are just boilerplate around items; it turns out indexedItems (defined below) can replace a number of these (or could be adapted to handle all of these)

proposal A

proposal B (simpler alternative, could be done independently of proposal A)

proof of concept

#[
requires:
{.experimental: "forLoopMacros".}
or
{.push experimental: "forLoopMacros".}
call...
{.pop.}
]#

import macros

macro indexedItems*(x: ForLoopStmt): untyped {.inline.} =
  expectKind x, nnkForStmt
  result = newStmtList()
  result.add newVarStmt(x[0], newLit(0))
  var body = x[^1]
  if body.kind != nnkStmtList:
    body = newTree(nnkStmtList, body)
  body.add newCall(bindSym"inc", x[0])
  var newFor = newTree(nnkForStmt)
  for i in 1..x.len-3:
    newFor.add x[i]
  newFor.add x[^2][1]
  newFor.add body
  result.add newFor
  result = quote do:
    block:
      `result`

iterator indexedItems*[IX, T](a: array[IX, T]): tuple[key: IX, val: T] {.inline.} =
  ## iterates over each item of `a`. Yields ``(index, a[index])`` pairs.
  var i = low(IX)
  if i <= high(IX):
    while true:
      yield (i, a[i])
      if i >= high(IX): break
      inc(i)

when true:
  proc test_indexedItems=
    {.push experimental: "forLoopMacros".}
    for i,a in indexedItems(@[10,11,12]):
      echo (i, a)
    # checks that calling twice doesn't cause redefinition of variables
    for i,a in indexedItems(@[10,11,12]):
      echo (i, a)

    var arr: array[20..22, int] = [10,11,12]
    for i,a in indexedItems(arr):
      echo (i, a)
    {.pop.}
  test_indexedItems()

caveat

as example above shows, macro indexedItems can't be overloaded by iterator indexedItems so maybe the above code could be made more robust, or we can just call the macro indexedItems

note

this seems like a really useful use of forLoopMacros ; could we make forLoopMacros enabled by default now that it's been out in the wild for a while?

benefit

krux02 commented 5 years ago

well, I have to say I like it. There are too many iterator definitions necessary to make a collection type usable.

I am not sure though if this is the best solution.

Caveats are. importing anything (including macros) in system.nim is forbidden. Therefore the macro would be required to be implemented in the compiler not as a macro.

Maybe the reverse is better. Just define pairs, not items and let the compiler figure out the existence of items.

Araq commented 5 years ago

I think an indexedItems in sugar.nim that uses ForLoop macros is the best option. Python calls it enumerate though, maybe worth to copy this name.

mratsim commented 5 years ago

I'm not sure which one I prefer, indexeditems is clearer but for discoverability enumerate is better, maybe use indexedItems and make sure to mention "This is equivalent to enumerate in other programming languages".

krux02 commented 5 years ago

Scala calls it zipWithIndex and then the index is the second value of the tuple. No I don't want that, but I would like to point out that there is no consistent naming across languages. I don't like the name enumerate, because it does not relate to enums at all.

alehander92 commented 5 years ago

Isn't there a way to make it "just work" with the current idiom? (for i, j in k) Requiring additional explicit indexedItems is a downgrade imo.

bluenote10 commented 5 years ago

@alehander42 I found the implicit pairs idiom quite confusing when learning Nim and I still have problems with it. For example I always struggle when I want to combine an enumeration index with an existing pairs iterator e.g. of a key-value like data structure like a table. In Python and Scala I have not problem writing such loops, because everything is explicit and the pairs call is not implicitly triggered by the number of loop arguments.

Some Python examples that I often have trouble writing in Nim:

d = {1: 1, 2: 2}

for k, v in d.items(): ...

for i, k in enumerate(d.keys()): ...

for i, v in enumerate(d.values()): ...

for i, (k, v) in enumerate(d.items()): ...

I guess the implicit pairs call is the reason why the first three are difficult to differentiate in Nim.

I would assume that this is more straightforward with an explicit indexedItems iterator like it is in Python/Scala.

andreaferretti commented 5 years ago

Well, in Nim it should be enough to do

let d = {1: 1, 2: 2}.initTable

for k, v in d: ...

for i, k in d.keys: ...

for i, v in d.values: ...

which seems to me even simpler - no need to recall items(), iteritems(), enumerate() and the differences thereof

bluenote10 commented 5 years ago

@andreaferretti Unfortunately that does not work. Versions 2 and 3 fail to compile because of Error: wrong number of variables. And how would you write example 4?

To me the explicit calls to e.g. enumerate, items, keys, or values makes it much clearer what the code does. It is very unusual that the left part of the in affects the expression on the right hand side. I would expect that this comes as a surprise to most people learning Nim. The intuitive expectation is that the expression on the left has to match the iterator type.

andreaferretti commented 5 years ago

I admit I did not try these. For the fourth example, it is not writable of course without forLoop macros, but that has nothing to do with pairs - it is just this issue

narimiran commented 5 years ago

And how would you write example 4?

Hopefully, one day we'll have this: #7486

metagn commented 3 years ago

Now that we have enumerate, should this be closed or should it be kept as an RFC to remove the pairs overloads?

timotheecour commented 3 years ago