fix #14404 foldr had the classic multiple evaluation bug

[timotheecour]

fix #14404

after PR

move test to tests/stdlib/tsequtils.nim like most modules; rationale:

• this makes modules more lightweight to import for normal usage (eg for parser)
• joinable via megatest
• avoids unintentional bugs that would only be triggered when testing code via an import as opposed to being in same module (eg some symbol being not exported or causing a clash otherwise)

note that isMainModule still has its uses, just not for that

current implementation is not efficient: seq copy un-needed

let s = sequence makes an un-necessary copy; {.evalonce.} (#13750) would fix that

1 redundant copy per iteration

this would save 1 copy per iteration:

  block:
    var b {.inject.} = s[n - 1]
    for i in countdown(n - 2, 0):
      let a {.inject.} = s[i]
      b = operation
    b

use better tests

foldl/foldr were introduced in 2013, since then nim has improved

• tests should used doAssert, not assert (otherwise testing via -d:release is pointless)
• out of line asserts (let block followed by assert block) are more verbose and less readable than inline asserts
• errmsgs are redundant
  so I'll rewrite as:

  block: # foldr tests
    let
      numbers = @[5, 9, 11]
      addition = foldr(numbers, a + b)
      subtraction = foldr(numbers, a - b)
      multiplication = foldr(numbers, a * b)
      words = @["nim", "is", "cool"]
      concatenation = foldr(words, a & b)
    assert addition == 25, "Addition is (5+(9+(11)))"
    assert subtraction == 7, "Subtraction is (5-(9-(11)))"
    assert multiplication == 495, "Multiplication is (5*(9*(11)))"
    assert concatenation == "nimiscool"
    doAssert toSeq(1..3).foldr(a + b) == 6 # issue #14404
=>
  block: # foldr tests
    let numbers = @[5, 9, 11]
    doAssert foldr(numbers, a + b) == (5+(9+(11)))
    doAssert foldr(numbers, a - b) == (5-(9-(11)))
    doAssert foldr(numbers, a * b) == (5*(9*(11)))
    doAssert foldr(@["nim", "is", "cool"], a & b) == "nimiscool"
    doAssert toSeq(1..3).foldr(a + b) == 6 # issue #14404

or even use unittest.check, with obvious benefits

• tests and examples should not be redundant
  there's zero difference between doAssert foldr(numbers, a + b) == (5+(9+(11))) and doAssert foldr(numbers, a * b) == (5*(9*(11))) ; only 1 of these is needed (zero difference = same code path, no corner case); unlike many procs in os.nim which need more testing and more examples because they're full of corner cases that need to be clarified

[timotheecour]

@Araq PR ready, and please let me know if there's anything you disagree with in the "after PR" points before I work on it (the evalonce must be delayed till I update #13750 though)

[Araq]

please let me know if there's anything you disagree with in the "after PR" points before I work on it

I agree with all of your points.

[Clyybber]

@timotheecour Can you explain how this PR fixed the root cause of #14404 ?
I like this PR and think its a good idea to prevent multiple evaluations, but I think #14404 might have another bug as the root issue (because its triggering depends on wether method call syntax is used or not)

[timotheecour]

@Clyybber ya sorry for late reply, you're right, also it shouldn't depend on whether it's at proc or module level; please file a bug (ideally with no module dependency!)

[timotheecour]

please file a bug (ideally with no module dependency!)

=> #14844