From d855c5442ff259ddde52765af2ba02e31db8f275 Mon Sep 17 00:00:00 2001
From: metagn <10591326+metagn@users.noreply.github.com>
Date: Wed, 23 Aug 2023 09:56:22 +0300
Subject: [PATCH 1/3] round out tuple unpacking assignment, support underscores

fixes #18710
---
 compiler/lowerings.nim           | 19 ------------------
 compiler/semexprs.nim            | 33 +++++++++++++++++++++++++++++++-
 compiler/semstmts.nim            | 17 +++++++++-------
 tests/errmsgs/tassignunpack.nim  |  2 +-
 tests/tuples/ttuples_various.nim | 12 ++++++++++++
 5 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/compiler/lowerings.nim b/compiler/lowerings.nim
index a083b9195315..42d0f1790c0d 100644
--- a/compiler/lowerings.nim
+++ b/compiler/lowerings.nim
@@ -122,25 +122,6 @@ proc newTupleAccessRaw*(tup: PNode, i: int): PNode =
 proc newTryFinally*(body, final: PNode): PNode =
   result = newTree(nkHiddenTryStmt, body, newTree(nkFinally, final))
 
-proc lowerTupleUnpackingForAsgn*(g: ModuleGraph; n: PNode; idgen: IdGenerator; owner: PSym): PNode =
-  let value = n.lastSon
-  result = newNodeI(nkStmtList, n.info)
-
-  var temp = newSym(skTemp, getIdent(g.cache, "_"), idgen, owner, value.info, owner.options)
-  var v = newNodeI(nkLetSection, value.info)
-  let tempAsNode = newSymNode(temp) #newIdentNode(getIdent(genPrefix & $temp.id), value.info)
-
-  var vpart = newNodeI(nkIdentDefs, tempAsNode.info, 3)
-  vpart[0] = tempAsNode
-  vpart[1] = newNodeI(nkTupleClassTy, value.info)
-  vpart[2] = value
-  v.add vpart
-  result.add(v)
-
-  let lhs = n[0]
-  for i in 0..<lhs.len:
-    result.add newAsgnStmt(lhs[i], newTupleAccessRaw(tempAsNode, i))
-
 proc lowerSwap*(g: ModuleGraph; n: PNode; idgen: IdGenerator; owner: PSym): PNode =
   result = newNodeI(nkStmtList, n.info)
   # note: cannot use 'skTemp' here cause we really need the copy for the VM :-(
diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index 52d1f0628ea7..eb5fdf4e5e1c 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -1828,6 +1828,37 @@ proc goodLineInfo(arg: PNode): TLineInfo =
   else:
     arg.info
 
+proc makeTupleAssignments(c: PContext; n: PNode): PNode =
+  ## expand tuple unpacking assignment into series of assignments
+  ## 
+  ## mirrored with semstmts.makeVarTupleSection
+  let lhs = n[0]
+  let value = semExprWithType(c, n[1], {efTypeAllowed})
+  if value.typ.kind != tyTuple:
+    localError(c.config, n[1].info, errXExpected, "tuple")
+  elif lhs.len != value.typ.len:
+    localError(c.config, n.info, errWrongNumberOfVariables)
+  result = newNodeI(nkStmtList, n.info)
+
+  let temp = newSym(skTemp, getIdent(c.cache, "tmpTupleAsgn"), c.idgen, getCurrOwner(c), n.info)
+  temp.typ = value.typ
+  temp.flags.incl(sfGenSym)
+  var v = newNodeI(nkLetSection, value.info)
+  let tempNode = newSymNode(temp) #newIdentNode(getIdent(genPrefix & $temp.id), value.info)
+  var vpart = newNodeI(nkIdentDefs, v.info, 3)
+  vpart[0] = tempNode
+  vpart[1] = c.graph.emptyNode
+  vpart[2] = value
+  v.add vpart
+  result.add(v)
+
+  for i in 0..<lhs.len:
+    if lhs[i].kind == nkIdent and lhs[i].ident.id == ord(wUnderscore):
+      # skip _ assignments if we are using a temp as they are already evaluated
+      continue
+    else:
+      result.add newAsgnStmt(lhs[i], newTupleAccessRaw(tempNode, i))
+
 proc semAsgn(c: PContext, n: PNode; mode=asgnNormal): PNode =
   checkSonsLen(n, 2, c.config)
   var a = n[0]
@@ -1870,7 +1901,7 @@ proc semAsgn(c: PContext, n: PNode; mode=asgnNormal): PNode =
       # unfortunately we need to rewrite ``(x, y) = foo()`` already here so
       # that overloading of the assignment operator still works. Usually we
       # prefer to do these rewritings in transf.nim:
-      return semStmt(c, lowerTupleUnpackingForAsgn(c.graph, n, c.idgen, c.p.owner), {})
+      return semStmt(c, makeTupleAssignments(c, n), {})
     else:
       a = semExprWithType(c, a, {efLValue})
   else:
diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim
index 39c37f4fb25f..bb9c474a8f85 100644
--- a/compiler/semstmts.nim
+++ b/compiler/semstmts.nim
@@ -599,12 +599,14 @@ proc globalVarInitCheck(c: PContext, n: PNode) =
 
 proc makeVarTupleSection(c: PContext, n, a, def: PNode, typ: PType, symkind: TSymKind, origResult: var PNode): PNode =
   ## expand tuple unpacking assignments into new var/let/const section
+  ## 
+  ## mirrored with semexprs.makeTupleAssignments
   if typ.kind != tyTuple:
     localError(c.config, a.info, errXExpected, "tuple")
   elif a.len-2 != typ.len:
     localError(c.config, a.info, errWrongNumberOfVariables)
   var
-    tmpTuple: PSym = nil
+    tempNode: PNode = nil
     lastDef: PNode
   let defkind = if symkind == skConst: nkConstDef else: nkIdentDefs
   # temporary not needed if not const and RHS is tuple literal
@@ -612,17 +614,18 @@ proc makeVarTupleSection(c: PContext, n, a, def: PNode, typ: PType, symkind: TSy
   let useTemp = def.kind notin {nkPar, nkTupleConstr} or symkind == skConst
   if useTemp:
     # use same symkind for compatibility with original section
-    tmpTuple = newSym(symkind, getIdent(c.cache, "tmpTuple"), c.idgen, getCurrOwner(c), n.info)
-    tmpTuple.typ = typ
-    tmpTuple.flags.incl(sfGenSym)
+    let temp = newSym(symkind, getIdent(c.cache, "tmpTuple"), c.idgen, getCurrOwner(c), n.info)
+    temp.typ = typ
+    temp.flags.incl(sfGenSym)
     lastDef = newNodeI(defkind, a.info)
     newSons(lastDef, 3)
-    lastDef[0] = newSymNode(tmpTuple)
+    lastDef[0] = newSymNode(temp)
     # NOTE: at the moment this is always ast.emptyNode, see parser.nim
     lastDef[1] = a[^2]
     lastDef[2] = def
-    tmpTuple.ast = lastDef
+    temp.ast = lastDef
     addToVarSection(c, origResult, n, lastDef)
+    tempNode = newSymNode(temp)
   result = newNodeI(n.kind, a.info)
   for j in 0..<a.len-2:
     let name = a[j]
@@ -641,7 +644,7 @@ proc makeVarTupleSection(c: PContext, n, a, def: PNode, typ: PType, symkind: TSy
       lastDef[0] = name
     lastDef[^2] = c.graph.emptyNode
     if useTemp:
-      lastDef[^1] = newTreeIT(nkBracketExpr, name.info, typ[j], newSymNode(tmpTuple), newIntNode(nkIntLit, j))
+      lastDef[^1] = newTupleAccessRaw(tempNode, j)
     else:
       var val = def[j]
       if val.kind == nkExprColonExpr: val = val[1]
diff --git a/tests/errmsgs/tassignunpack.nim b/tests/errmsgs/tassignunpack.nim
index 27413a42b697..d74e16dd5e9f 100644
--- a/tests/errmsgs/tassignunpack.nim
+++ b/tests/errmsgs/tassignunpack.nim
@@ -1,3 +1,3 @@
 var a, b = 0
 (a, b) = 1 #[tt.Error
-         ^ type mismatch: got <int literal(1)> but expected 'tuple']#
+         ^ 'tuple' expected]#
diff --git a/tests/tuples/ttuples_various.nim b/tests/tuples/ttuples_various.nim
index 97bc70bd286d..e392731d2f0a 100644
--- a/tests/tuples/ttuples_various.nim
+++ b/tests/tuples/ttuples_various.nim
@@ -197,3 +197,15 @@ block: # bug #22054
 
   var v = A(field: (a: 1314))
   doAssert get(v)[0] == 1314
+
+block: # tuple unpacking assignment with underscore
+  var
+    a = 1
+    b = 2
+  doAssert (a, b) == (1, 2)
+  (a, _) = (3, 4)
+  doAssert (a, b) == (3, 2)
+  (_, a) = (5, 6)
+  doAssert (a, b) == (6, 2)
+  (b, _) = (7, 8)
+  doAssert (a, b) == (6, 7)

From bee77943ff5dfcbf91f80c7a2441e36db39620c9 Mon Sep 17 00:00:00 2001
From: metagn <10591326+metagn@users.noreply.github.com>
Date: Wed, 23 Aug 2023 10:51:13 +0300
Subject: [PATCH 2/3] fix test messages

---
 tests/arc/topt_no_cursor.nim      | 8 ++++----
 tests/types/tassignemptytuple.nim | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/arc/topt_no_cursor.nim b/tests/arc/topt_no_cursor.nim
index 7cfb0a0d50bd..dfb0f0a3867a 100644
--- a/tests/arc/topt_no_cursor.nim
+++ b/tests/arc/topt_no_cursor.nim
@@ -39,13 +39,13 @@ var
   lresult
   lvalue
   lnext
-  _
+  tmpTupleAsgn
 lresult = @[123]
-_ = (
+tmpTupleAsgn = (
   let blitTmp = lresult
   blitTmp, ";")
-lvalue = _[0]
-lnext = _[1]
+lvalue = tmpTupleAsgn[0]
+lnext = tmpTupleAsgn[1]
 `=sink`(result.value, move lvalue)
 `=destroy`(lnext)
 `=destroy_1`(lvalue)
diff --git a/tests/types/tassignemptytuple.nim b/tests/types/tassignemptytuple.nim
index f3320dec7ace..9d5a311baa32 100644
--- a/tests/types/tassignemptytuple.nim
+++ b/tests/types/tassignemptytuple.nim
@@ -1,5 +1,5 @@
 discard """
-  errormsg: "invalid type: 'empty' in this context: '(seq[empty], (seq[empty], set[empty]))' for let"
+  errormsg: "cannot infer the type of the tuple"
   file: "tassignemptytuple.nim"
   line: 11
 """

From b195b96274a8dbbb4d9d4b876c95af4b0a5812e1 Mon Sep 17 00:00:00 2001
From: metagn <metagngn@gmail.com>
Date: Thu, 24 Aug 2023 03:08:40 +0300
Subject: [PATCH 3/3] use discard instead of continue

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
---
 compiler/semexprs.nim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index eb5fdf4e5e1c..cb27ca0ff448 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -1855,7 +1855,7 @@ proc makeTupleAssignments(c: PContext; n: PNode): PNode =
   for i in 0..<lhs.len:
     if lhs[i].kind == nkIdent and lhs[i].ident.id == ord(wUnderscore):
       # skip _ assignments if we are using a temp as they are already evaluated
-      continue
+      discard
     else:
       result.add newAsgnStmt(lhs[i], newTupleAccessRaw(tempNode, i))