fixes #8577, #8580, other bug fixes

lib/pure/collections/sequtils.nim

@@ -25,6 +25,22 @@ import macros
 when not defined(nimhygiene):
   {.pragma: dirty.}
 
+
+macro evalOnce(v, exp: untyped, letAssigneable: static[bool]): untyped =
+  expectKind(v, nnkIdent)
+  var val = exp
+
+  result = newStmtList()
+  # If `exp` is not a symbol we evaluate it once here and then use the temporary
+  # symbol as alias
+  if exp.kind != nnkSym and letAssigneable:
+    val = genSym()
+    result.add(newLetStmt(val, exp))
+
+  result.add(
+    newProc(name = genSym(nskTemplate, $v), params = [getType(untyped)],
+      body = val, procType = nnkTemplateDef))
+
 proc concat*[T](seqs: varargs[seq[T]]): seq[T] =
   ## Takes several sequences' items and returns them inside a new sequence.
   ##
@@ -635,30 +651,7 @@ template mapIt*(s, typ, op: untyped): untyped =
     result.add(op)
   result
 
-# This is needed in order not to break the bootstrap, the fallback
-# implementation is a "dumb" let that won't work in some cases (eg. when `exp`
-# is an openArray)
-when declared(macros.symKind):
-  macro evalOnce(v, exp: untyped): untyped =
-    expectKind(v, nnkIdent)
-    var val = exp
-
-    result = newStmtList()
-
-    # Not a parameter we can pass as-is, evaluate and store in a temporary
-    # variable
-    if exp.kind != nnkSym or exp.symKind != nskParam:
-      val = genSym()
-      result.add(newLetStmt(val, exp))
-
-    result.add(
-      newProc(name = genSym(nskTemplate, $v), params = [getType(untyped)],
-        body = val, procType = nnkTemplateDef))
-else:
-  macro evalOnce(v, exp: untyped): untyped =
-    result = newLetStmt(v, exp)
-
-template mapIt*(s, op: untyped): untyped =
+template mapIt*(s: typed, op: untyped): untyped =
   ## Convenience template around the ``map`` proc to reduce typing.
   ##
   ## The template injects the ``it`` variable which you can use directly in an
@@ -675,19 +668,27 @@ template mapIt*(s, op: untyped): untyped =
     block:
       var it{.inject.}: type(items(s));
       op))
-  var result: seq[outType]
   when compiles(s.len):
-    evalOnce(t, s)
-    var i = 0
-    result = newSeq[outType](t.len)
-    for it {.inject.} in t:
-      result[i] = op
-      i += 1
+    block: # using a block avoids https://github.com/nim-lang/Nim/issues/8580
+
+      # avoid double substitution of `s`.
+      # NOTE: if we used `type(s) isnot openArray` instead, the openarray
+      # literal would be substituted twice
+      # BUG: `evalOnce(t, s, false)` would lead to C compile errors
+      # (`error: use of undeclared identifier`) instead of Nim compile errors
+      evalOnce(t, s, compiles((let _ = s)))
+
+      var i = 0
+      var result = newSeq[outType](t.len)
+      for it {.inject.} in t:
+        result[i] = op
+        i += 1
+      result
   else:
-    result = @[]
+    var result: seq[outType] = @[]
     for it {.inject.} in s:
       result.add(op)
-  result
+    result
 
 template applyIt*(varSeq, op: untyped) =
   ## Convenience template around the mutable ``apply`` proc to reduce typing.
@@ -1067,9 +1068,30 @@ when isMainModule:
     doAssert mapLiterals(([1], ("abc"), 2), `$`, nested=true) == (["1"], "abc", "2")
 
   block: # mapIt with openArray
-    when declared(macros.symKind):
-      proc foo(x: openArray[int]): seq[int] = x.mapIt(it + 1)
-      doAssert foo([1,2,3]) == @[2,3,4]
+    proc foo(x: openArray[int]): seq[int] = x.mapIt(it + 1)
+    doAssert foo([1,2,3]) == @[2,3,4]
+
+  block: # mapIt with direct openArray
+    # see https://github.com/_render_node/MDExOlB1bGxSZXF1ZXN0MjA3MTQ3MTIw/pull_requests/timeline#issuecomment-411614711
+    doAssert openArray[int]([1,2]).mapIt(it) == @[1,2]
+
+  block: # mapIt test, see https://github.com/nim-lang/Nim/pull/8584#pullrequestreview-144723468
+    # NOTE: `[].mapIt(it)` is illegal, just as `let a = @[]` is
+    doAssert newSeq[int](0).mapIt(it) == @[]
+
+  block: # mapIt redifinition check, see https://github.com/nim-lang/Nim/issues/8580
+    let t = [1,2].mapIt(it)
+    doAssert t == @[1,2]
+
+  block:
+    var counter = 0
+    proc getInput():auto =
+      counter.inc
+      [1, 2]
+    doAssert getInput().mapIt(it*2).mapIt(it*10) == @[20, 40]
+    # make sure argument evaluated only once, analog to
+    # https://github.com/nim-lang/Nim/issues/7187 test case
+    doAssert counter == 1
 
   block: # mapIt with invalid RHS for `let` (#8566)
     type X = enum