From 63d1a0289e872212e836559f92a785df51f4faac Mon Sep 17 00:00:00 2001
From: Timothee Cour <timothee.cour2@gmail.com>
Date: Fri, 29 May 2020 08:10:59 -0700
Subject: [PATCH] fix #14421 items uses lent T (#14447)

* fix #14421 items uses lent T for seq + openArray
* add -d:nimWorkaround14447
* fix test
---
 compiler/condsyms.nim                   |  1 +
 compiler/lambdalifting.nim              |  3 ++-
 lib/system/iterators.nim                | 31 ++++++++++++++++++++++---
 testament/important_packages.nim        |  4 ++--
 tests/destructor/tbintree2.nim          |  2 +-
 tests/destructor/tgcdestructors.nim     |  2 +-
 tests/destructor/tnewruntime_misc.nim   |  2 +-
 tests/destructor/towned_binary_tree.nim |  2 +-
 tests/destructor/tsimpleclosure.nim     |  2 +-
 tests/destructor/tv2_raise.nim          |  2 +-
 tests/destructor/twidgets_unown.nim     |  2 +-
 11 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/compiler/condsyms.nim b/compiler/condsyms.nim
index 7b05faa87afed..b7f7100f123ce 100644
--- a/compiler/condsyms.nim
+++ b/compiler/condsyms.nim
@@ -118,3 +118,4 @@ proc initDefines*(symbols: StringTableRef) =
   defineSymbol("nimHasInvariant")
   defineSymbol("nimHasStacktraceMsgs")
   defineSymbol("nimDoesntTrackDefects")
+  defineSymbol("nimHasLentIterators")
diff --git a/compiler/lambdalifting.nim b/compiler/lambdalifting.nim
index fc1ad5e4033a3..c1cd075837989 100644
--- a/compiler/lambdalifting.nim
+++ b/compiler/lambdalifting.nim
@@ -291,7 +291,8 @@ proc markAsClosure(g: ModuleGraph; owner: PSym; n: PNode) =
   if illegalCapture(s):
     localError(g.config, n.info,
       ("'$1' is of type <$2> which cannot be captured as it would violate memory" &
-       " safety, declared here: $3") % [s.name.s, typeToString(s.typ), g.config$s.info])
+       " safety, declared here: $3; using '-d:nimWorkaround14447' helps in some cases") %
+      [s.name.s, typeToString(s.typ), g.config$s.info])
   elif owner.typ.callConv notin {ccClosure, ccDefault}:
     localError(g.config, n.info, "illegal capture '$1' because '$2' has the calling convention: <$3>" %
       [s.name.s, owner.name.s, CallingConvToStr[owner.typ.callConv]])
diff --git a/lib/system/iterators.nim b/lib/system/iterators.nim
index 53b287db000aa..5d8685e409678 100644
--- a/lib/system/iterators.nim
+++ b/lib/system/iterators.nim
@@ -1,7 +1,32 @@
-iterator items*[T](a: openArray[T]): T {.inline.} =
+when defined(nimHasLentIterators) and not defined(nimWorkaround14447):
+  template lent2(T): untyped =
+    # xxx this should actually depend on T.sizeof >= thresLentSizeof
+    # with for example `thresLentSizeof ~= int.sizeof`:
+    # it may be faster to return by value for small sizes compared to
+    # forcing a deref; this could be adjusted using profiling.
+    # However, `simply using `when T.sizeof >= thresLentSizeof: lent T else: T`
+    # does not work, for a few reasons (eg importc types would cause CT error
+    # and we can't filter them out without compiles() or some magic.
+    lent T
+else:
+  template lent2(T): untyped = T
+
+iterator items*[T: not char](a: openArray[T]): lent2 T {.inline.} =
   ## Iterates over each item of `a`.
   var i = 0
-  while i < len(a):
+  let n = len(a)
+  while i < n:
+    yield a[i]
+    inc(i)
+
+iterator items*[T: char](a: openArray[T]): T {.inline.} =
+  ## Iterates over each item of `a`.
+  # a VM bug currently prevents taking address of openArray[char]
+  # elements converted from a string (would fail in `tests/misc/thallo.nim`)
+  # in any case there's no performance advantage of returning char by address.
+  var i = 0
+  let n = len(a)
+  while i < n:
     yield a[i]
     inc(i)
 
@@ -179,7 +204,7 @@ iterator mpairs*(a: var cstring): tuple[key: int, val: var char] {.inline.} =
       yield (i, a[i])
       inc(i)
 
-iterator items*[T](a: seq[T]): T {.inline.} =
+iterator items*[T](a: seq[T]): lent2 T {.inline.} =
   ## Iterates over each item of `a`.
   var i = 0
   let L = len(a)
diff --git a/testament/important_packages.nim b/testament/important_packages.nim
index 1d3b1917c5eb7..cb436bfb2f81e 100644
--- a/testament/important_packages.nim
+++ b/testament/important_packages.nim
@@ -43,7 +43,7 @@ pkg1 "elvis"
 pkg1 "fidget", true, "nim c -d:release -r tests/runNative.nim"
 pkg1 "fragments", false, "nim c -r fragments/dsl.nim"
 pkg1 "gara"
-pkg1 "ggplotnim", true, "nim c -d:noCairo -r tests/tests.nim"
+pkg1 "ggplotnim", true, "nim c -d:noCairo -r -d:nimWorkaround14447 tests/tests.nim"
 # pkg1 "gittyup", true, "nimble test", "https://github.com/disruptek/gittyup"
 pkg1 "glob"
 pkg1 "gnuplot"
@@ -134,4 +134,4 @@ pkg2 "websocket", false, "nim c websocket.nim"
 pkg2 "with"
 pkg2 "ws"
 pkg2 "yaml"
-pkg2 "zero_functional", false, "nim c -r test.nim"
+pkg2 "zero_functional", false, "nim c -r -d:nimWorkaround14447 test.nim"
diff --git a/tests/destructor/tbintree2.nim b/tests/destructor/tbintree2.nim
index e910f430a0a7a..6fdda6e546671 100644
--- a/tests/destructor/tbintree2.nim
+++ b/tests/destructor/tbintree2.nim
@@ -1,7 +1,7 @@
 discard """
   cmd: '''nim c -d:nimAllocStats --newruntime $file'''
   output: '''0
-(allocCount: 6, deallocCount: 6)'''
+(allocCount: 5, deallocCount: 5)'''
 """
 
 import system / ansi_c
diff --git a/tests/destructor/tgcdestructors.nim b/tests/destructor/tgcdestructors.nim
index 61d296d2edbc4..4731bf694f1fb 100644
--- a/tests/destructor/tgcdestructors.nim
+++ b/tests/destructor/tgcdestructors.nim
@@ -10,7 +10,7 @@ a: @[4, 2, 3]
 0
 30
 true
-(allocCount: 36, deallocCount: 36)'''
+(allocCount: 27, deallocCount: 27)'''
 """
 
 include system / ansi_c
diff --git a/tests/destructor/tnewruntime_misc.nim b/tests/destructor/tnewruntime_misc.nim
index f6e2a6157a651..1a5b0e3b0f1d2 100644
--- a/tests/destructor/tnewruntime_misc.nim
+++ b/tests/destructor/tnewruntime_misc.nim
@@ -7,7 +7,7 @@ axc
 ...
 destroying GenericObj[T] GenericObj[system.int]
 test
-(allocCount: 17, deallocCount: 15)
+(allocCount: 13, deallocCount: 11)
 3'''
 """
 
diff --git a/tests/destructor/towned_binary_tree.nim b/tests/destructor/towned_binary_tree.nim
index 72f7cae0cd595..320e33759ecdb 100644
--- a/tests/destructor/towned_binary_tree.nim
+++ b/tests/destructor/towned_binary_tree.nim
@@ -1,7 +1,7 @@
 discard """
   cmd: '''nim c -d:nimAllocStats --newruntime $file'''
   output: '''31665
-(allocCount: 33335, deallocCount: 33335)'''
+(allocCount: 33334, deallocCount: 33334)'''
 """
 
 #  bug #11053
diff --git a/tests/destructor/tsimpleclosure.nim b/tests/destructor/tsimpleclosure.nim
index 4916f4bab583e..35c57a63420f7 100644
--- a/tests/destructor/tsimpleclosure.nim
+++ b/tests/destructor/tsimpleclosure.nim
@@ -5,7 +5,7 @@ discard """
 hello
 hello
 hello
-(allocCount: 4, deallocCount: 4)'''
+(allocCount: 3, deallocCount: 3)'''
 """
 
 import system / ansi_c
diff --git a/tests/destructor/tv2_raise.nim b/tests/destructor/tv2_raise.nim
index 4ea94a864a3b2..66b0aec303435 100644
--- a/tests/destructor/tv2_raise.nim
+++ b/tests/destructor/tv2_raise.nim
@@ -2,7 +2,7 @@ discard """
   valgrind: true
   cmd: '''nim c -d:nimAllocStats --newruntime $file'''
   output: '''OK 3
-(allocCount: 8, deallocCount: 5)'''
+(allocCount: 7, deallocCount: 4)'''
 """
 
 import strutils, math
diff --git a/tests/destructor/twidgets_unown.nim b/tests/destructor/twidgets_unown.nim
index 3a3c4c66573e6..8653d5c28dfe8 100644
--- a/tests/destructor/twidgets_unown.nim
+++ b/tests/destructor/twidgets_unown.nim
@@ -2,7 +2,7 @@ discard """
   cmd: '''nim c -d:nimAllocStats --newruntime $file'''
   output: '''button
 clicked!
-(allocCount: 7, deallocCount: 7)'''
+(allocCount: 6, deallocCount: 6)'''
 """
 
 import system / ansi_c