From 5330cd789110f4c3072a48c83923860851ded6cb Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Wed, 15 Feb 2023 16:45:53 +0100
Subject: [PATCH] disallow `sink openArray[T]` types

## Summary
No specification for how `sink openArray[T]` should work exists, and
the current implementation only works without issues in the simple case
where the argument is a literal array-constructor expression that is not
constant.

For most other cases, either the `seq` or `string` used as the argument
are not cleaned up or, for constant expression, a segmentation fault
occurs at run-time when trying to mutate the `openArray`.

Passing an immutable `openArray` to a `sink openArray`, while in theory
supported, also results in a run-time error.

## Details
- remove the "cannot create implicit openArray` report; it's unused now
- disable the test that made use of `sink openArray`
---
 compiler/ast/report_enums.nim                     |  1 -
 compiler/front/cli_reporter.nim                   |  3 ---
 compiler/sem/injectdestructors.nim                |  4 ----
 compiler/sem/typeallowed.nim                      |  3 ++-
 tests/lang_objects/destructor/tmove_objconstr.nim | 10 +++++-----
 5 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/compiler/ast/report_enums.nim b/compiler/ast/report_enums.nim
index a55ed7eabca..56a6d038bac 100644
--- a/compiler/ast/report_enums.nim
+++ b/compiler/ast/report_enums.nim
@@ -629,7 +629,6 @@ type
     rsemCannotMixTypesAndValuesInTuple
     rsemExpectedTypelessDeferBody
     rsemInvalidBindContext
-    rsemCannotCreateImplicitOpenarray
     rsemCannotAssignToDiscriminantWithCustomDestructor
     rsemUnavailableTypeBound
 
diff --git a/compiler/front/cli_reporter.nim b/compiler/front/cli_reporter.nim
index ee867fdf015..f7e40e8316c 100644
--- a/compiler/front/cli_reporter.nim
+++ b/compiler/front/cli_reporter.nim
@@ -2056,9 +2056,6 @@ proc reportBody*(conf: ConfigRef, r: SemReport): string =
         "destructor.\nIt is best to factor out piece of object that needs " &
         "custom destructor into separate object or not use discriminator assignment"
 
-    of rsemCannotCreateImplicitOpenarray:
-      result = "cannot create an implicit openArray copy to be passed to a sink parameter"
-
     of rsemWrongNumberOfQuoteArguments:
       assert false, "UNUSED"
 
diff --git a/compiler/sem/injectdestructors.nim b/compiler/sem/injectdestructors.nim
index 16cc0c279f9..7de3a7b8d3a 100644
--- a/compiler/sem/injectdestructors.nim
+++ b/compiler/sem/injectdestructors.nim
@@ -492,10 +492,6 @@ proc passCopyToSink(n: PNode; c: var Con; s: var Scope): PNode =
     if c.graph.config.selectedGC in {gcArc, gcOrc}:
       assert(not containsManagedMemory(n.typ))
 
-    if n.typ.skipTypes(abstractInst).kind in {tyOpenArray, tyVarargs}:
-      localReport(c.graph.config, n.info, reportAst(
-        rsemCannotCreateImplicitOpenarray, n))
-
     result.add newTree(nkAsgn, tmp, p(n, c, s, normal))
   # Since we know somebody will take over the produced copy, there is
   # no need to destroy it.
diff --git a/compiler/sem/typeallowed.nim b/compiler/sem/typeallowed.nim
index c8c50561512..6af62529e86 100644
--- a/compiler/sem/typeallowed.nim
+++ b/compiler/sem/typeallowed.nim
@@ -142,7 +142,8 @@ proc typeAllowedAux(marker: var IntSet, typ: PType, kind: TSymKind,
       result = typeAllowedAux(marker, t[0], kind, c, flags+{taIsOpenArray})
   of tySink:
     # you cannot nest openArrays/sinks/etc.
-    if kind != skParam or taIsOpenArray in flags or t[0].kind in {tySink, tyLent, tyVar}:
+    if kind != skParam or taIsOpenArray in flags or
+       t[0].kind in {tySink, tyLent, tyVar, tyOpenArray}:
       result = t
     else:
       result = typeAllowedAux(marker, t[0], kind, c, flags)
diff --git a/tests/lang_objects/destructor/tmove_objconstr.nim b/tests/lang_objects/destructor/tmove_objconstr.nim
index 5faaabb8b0e..7fb98668f56 100644
--- a/tests/lang_objects/destructor/tmove_objconstr.nim
+++ b/tests/lang_objects/destructor/tmove_objconstr.nim
@@ -185,9 +185,9 @@ type
   TableNonCopyable = object
     x: seq[(string, MySeqNonCopyable)]
 
-proc toTable(pairs: sink openArray[(string, MySeqNonCopyable)]): TableNonCopyable =
-  discard
-
-
-let mytable = {"a": newMySeq(2, 5.0)}.toTable
+# XXX: ``sink openArray[T]`` is currently disallowed
+when false:
+  proc toTable(pairs: sink openArray[(string, MySeqNonCopyable)]): TableNonCopyable =
+    discard
 
+  let mytable = {"a": newMySeq(2, 5.0)}.toTable