fix #13739

[krux02]

fixes #13739

circumvents #13726 by not addressing #13417

[timotheecour]

arrays will deep copy here (pretty bad).
circumvents #13726

can some performance regression similar to #13726 happen as a result? if yes, I think we should fix it differently, using first class openarray (nim-lang/RFCs#88); I have a POC on that.

[krux02]

can some performance regression similar to #13726 happen as a result? if yes, I think we should fix it differently, using first class openarray (nim-lang/RFCs#88); I have a POC on that.

Currently the input argument is someties copied into a temporary, and sometimes it is evaluated multiple times. The rules here are arbitrary. If you optimize for once case you make anther case slower. In this PR I kept the arbitrary rules. So I don't think a performance regression will happen. A deep copy only happens when there was a deep copy before. But now it doesn't create invalid code for arrays.

If first class openArray values will solve the problen still has to be proven. As far as I know, openarrays can only reference values. But for the temporary a move assignment is often necessary.

[timotheecour]

So I don't think a performance regression will happen. A deep copy only happens when there was a deep copy before

that's not what I'm observing
nim: 46c827b (devel): 4.000000000004e-06
nim2: 8368789 (your PR) 10.571572 (seconds)

when true: # D20200325T033719
  let n = 10_000_0000
  let m = 100
  var s = newSeq[int](n)
  for i in 0..<s.len: s[i] = i
  proc p2(): var seq[int] = s

  iterator ip1(v: openArray[int]): auto =
    yield v[0]
    yield v[1]
    yield v[^1]

  import times
  let t = cpuTime()
  for i in 0..<m:
    for x in ip1((discard; p2())):
      doAssert x != -1
  echo cpuTime() - t

how about this instead:

given this:

for x in myiter(body1): body2

when body1 is not a symbol, nim compiler transforms it into:

let tmp {.evalonce, gensym.} = body1
for x in myiter(tmp): body2

where {.evalonce.} is introduced in #13750; evalonce seems to provide a nicer alternative to evalOnceAs enabled by the recent feature introduced in #13508

this works with both lvalues and rvalues, doesn't make un-necessary copies, and enforces single evaluation.

[krux02]

@Araq ready for review. Test failure unrelated. @timotheecour 's remark has been addressed.