fixes #23247; don't destroy openarray since it doesn't own the data (#23254)

fixes #23247
closes #23251 (which accounts for why the openarray type is lifted
because ops are lifted for openarray conversions)

related: https://github.com/nim-lang/Nim/pull/18713

It seems to me that openarray doesn't own the data, so it cannot destroy
itself. The same case should be applied to
https://github.com/nim-lang/Nim/issues/19435. It shouldn't be destroyed
even openarray can have a destructor. A cleanup will be followed for
https://github.com/nim-lang/Nim/pull/19723 if it makes sense.

According to https://github.com/nim-lang/Nim/pull/12073, it lifts
destructor for openarray when openarray is sunk into the function, when
means `sink openarray` owns the data and needs to destroy it. In other
cases, destructor shouldn't be lifted for `openarray` in the first place
and it shouldn't destroy the data if it doesn't own it.

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>

compiler/injectdestructors.nim

@@ -594,7 +594,9 @@ template processScopeExpr(c: var Con; s: var Scope; ret: PNode, processCall: unt
   # tricky because you would have to intercept moveOrCopy at a certain point
   let tmp = c.getTemp(s.parent[], ret.typ, ret.info)
   tmp.sym.flags = tmpFlags
-  let cpy = if hasDestructor(c, ret.typ):
+  let cpy = if hasDestructor(c, ret.typ) and
+                ret.typ.kind notin {tyOpenArray, tyVarargs}:
+                # bug #23247 we don't own the data, so it's harmful to destroy it
               s.parent[].final.add c.genDestroy(tmp)
               moveOrCopy(tmp, ret, c, s, {IsDecl})
             else:
@@ -907,7 +909,10 @@ proc p(n: PNode; c: var Con; s: var Scope; mode: ProcessMode; tmpFlags = {sfSing
         result[0] = p(n[0], c, s, normal)
       if canRaise(n[0]): s.needsTry = true
       if mode == normal:
-        result = ensureDestruction(result, n, c, s)
+        if result.typ != nil and result.typ.kind notin {tyOpenArray, tyVarargs}:
+          # Returns of openarray types shouldn't be destroyed
+          # bug #19435; # bug #23247
+          result = ensureDestruction(result, n, c, s)
     of nkDiscardStmt: # Small optimization
       result = shallowCopy(n)
       if n[0].kind != nkEmpty:

tests/arc/t23247.nim

@@ -0,0 +1,52 @@
+discard """
+  matrix: ";-d:useMalloc"
+"""
+
+# bug #23247
+import std/hashes
+
+func baseAddr[T](x: openArray[T]): ptr T =
+  # Return the address of the zero:th element of x or `nil` if x is empty
+  if x.len == 0: nil else: cast[ptr T](x)
+
+func makeUncheckedArray[T](p: ptr T): ptr UncheckedArray[T] =
+  cast[ptr UncheckedArray[T]](p)
+
+type
+  LabelKey = object
+    data: seq[string]
+    refs: ptr UncheckedArray[string]
+    refslen: int
+
+  Gauge = ref object
+    metrics: seq[seq[seq[string]]]
+
+template values(key: LabelKey): openArray[string] =
+  if key.refslen > 0:
+    key.refs.toOpenArray(0, key.refslen - 1)
+  else:
+    key.data
+
+proc hash(key: LabelKey): Hash =
+  hash(key.values)
+
+proc view(T: type LabelKey, values: openArray[string]): T =
+  # TODO some day, we might get view types - until then..
+  LabelKey(refs: baseAddr(values).makeUncheckedArray(), refslen: values.len())
+
+template withValue2(k: untyped) =
+  discard hash(k)
+
+proc setGauge(
+    collector: Gauge,
+    labelValues: openArray[string],
+) =
+  let v = LabelKey.view(labelValues)
+  withValue2(v)
+  collector.metrics.add @[@labelValues, @labelValues]
+  discard @labelValues
+
+var nim_gc_mem_bytes = Gauge()
+let threadID = $getThreadId()
+setGauge(nim_gc_mem_bytes, @[threadID])
+setGauge(nim_gc_mem_bytes, @[threadID])