Merge #544

544: disallow `sink openArray[T]` types r=saem a=zerbina

## 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`



Co-authored-by: zerbina <[email protected]>

compiler/ast/report_enums.nim

@@ -629,7 +629,6 @@ type
     rsemCannotMixTypesAndValuesInTuple
     rsemExpectedTypelessDeferBody
     rsemInvalidBindContext
-    rsemCannotCreateImplicitOpenarray
     rsemCannotAssignToDiscriminantWithCustomDestructor
     rsemUnavailableTypeBound
 

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"
 

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.

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)

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