From 2315b01ae691e5e9e54fdfdfb4642c8fbc559e48 Mon Sep 17 00:00:00 2001 From: metagn Date: Tue, 28 Mar 2023 18:52:23 +0300 Subject: [PATCH] tuple unpacking for vars as just sugar, allowing nesting (#21563) * tuple unpacking for vars as just sugar, allowing nesting * set temp symbol AST * hopeful fix some issues, add test for #19364 * always use temp for consts * document, fix small issue * fix manual indentation * actually fix manual * use helper proc * don't resem temp tuple assignment --- changelogs/changelog_2_0_0.md | 20 ++++ compiler/parser.nim | 12 +- compiler/pipelines.nim | 2 +- compiler/semexprs.nim | 2 +- compiler/semstmts.nim | 193 +++++++++++++++++------------- doc/grammar.txt | 3 +- doc/manual.md | 33 ++++- tests/arc/t19364.nim | 30 +++++ tests/macros/tastrepr.nim | 4 +- tests/misc/t11634.nim | 3 - tests/parser/ttupleunpack.nim | 48 ++++++++ tests/tuples/mnimsconstunpack.nim | 4 + tests/tuples/tnimsconstunpack.nim | 8 ++ 13 files changed, 267 insertions(+), 95 deletions(-) create mode 100644 tests/arc/t19364.nim create mode 100644 tests/tuples/mnimsconstunpack.nim create mode 100644 tests/tuples/tnimsconstunpack.nim diff --git a/changelogs/changelog_2_0_0.md b/changelogs/changelog_2_0_0.md index cbe7554785..19d97b7690 100644 --- a/changelogs/changelog_2_0_0.md +++ b/changelogs/changelog_2_0_0.md @@ -343,6 +343,26 @@ - `=wasMoved` can be overridden by users. +- Tuple unpacking for variables is now treated as syntax sugar that directly + expands into multiple assignments. Along with this, tuple unpacking for + variables can now be nested. + + ```nim + proc returnsNestedTuple(): (int, (int, int), int, int) = (4, (5, 7), 2, 3) + + let (x, (_, y), _, z) = returnsNestedTuple() + # roughly becomes + let + tmpTup1 = returnsNestedTuple() + x = tmpTup1[0] + tmpTup2 = tmpTup1[1] + y = tmpTup2[1] + z = tmpTup1[3] + ``` + + As a result `nnkVarTuple` nodes in variable sections will no longer be + reflected in `typed` AST. + ## Compiler changes - The `gc` switch has been renamed to `mm` ("memory management") in order to reflect the diff --git a/compiler/parser.nim b/compiler/parser.nim index abfc7b3239..c9b30204c1 100644 --- a/compiler/parser.nim +++ b/compiler/parser.nim @@ -2277,13 +2277,19 @@ proc parseTypeDef(p: var Parser): PNode = setEndInfo() proc parseVarTuple(p: var Parser): PNode = - #| varTuple = '(' optInd identWithPragma ^+ comma optPar ')' '=' optInd expr + #| varTupleLhs = '(' optInd (identWithPragma / varTupleLhs) ^+ comma optPar ')' + #| varTuple = varTupleLhs '=' optInd expr result = newNodeP(nkVarTuple, p) getTok(p) # skip '(' optInd(p, result) # progress guaranteed - while p.tok.tokType in {tkSymbol, tkAccent}: - var a = identWithPragma(p, allowDot=true) + while p.tok.tokType in {tkSymbol, tkAccent, tkParLe}: + var a: PNode + if p.tok.tokType == tkParLe: + a = parseVarTuple(p) + a.add(p.emptyNode) + else: + a = identWithPragma(p, allowDot=true) result.add(a) if p.tok.tokType != tkComma: break getTok(p) diff --git a/compiler/pipelines.nim b/compiler/pipelines.nim index 2bb0bc2473..90f6de5c09 100644 --- a/compiler/pipelines.nim +++ b/compiler/pipelines.nim @@ -1,6 +1,6 @@ import sem, cgen, modulegraphs, ast, llstream, parser, msgs, lineinfos, reorder, options, semdata, cgendata, modules, pathutils, - packages, syntaxes, depends, vm, pragmas, idents, lookups + packages, syntaxes, depends, vm, pragmas, idents, lookups, wordrecg import pipelineutils diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index d3c96ce3c4..7ba4099d3e 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -1824,7 +1824,7 @@ proc semAsgn(c: PContext, n: PNode; mode=asgnNormal): PNode = result.add(n[1]) return semExprNoType(c, result) of nkPar, nkTupleConstr: - if a.len >= 2: + if a.len >= 2 or a.kind == nkTupleConstr: # 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: diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index 6d1ce388dd..2aa953d938 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -583,6 +583,57 @@ proc globalVarInitCheck(c: PContext, n: PNode) = if n.isLocalVarSym or n.kind in nkCallKinds and usesLocalVar(n): localError(c.config, n.info, errCannotAssignToGlobal) +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 + 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 + lastDef: PNode + let defkind = if symkind == skConst: nkConstDef else: nkIdentDefs + # temporary not needed if not const and RHS is tuple literal + # const breaks with seqs without temporary + 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"), nextSymId c.idgen, getCurrOwner(c), n.info) + tmpTuple.typ = typ + tmpTuple.flags.incl(sfGenSym) + lastDef = newNodeI(defkind, a.info) + newSons(lastDef, 3) + lastDef[0] = newSymNode(tmpTuple) + # NOTE: at the moment this is always ast.emptyNode, see parser.nim + lastDef[1] = a[^2] + lastDef[2] = def + tmpTuple.ast = lastDef + addToVarSection(c, origResult, n, lastDef) + result = newNodeI(n.kind, a.info) + for j in 0.. 3: - message(c.config, a.info, warnEachIdentIsTuple) + # generate new section from tuple unpacking and embed it into this one + let assignments = makeVarTupleSection(c, n, a, def, tup, symkind, result) + let resSection = semVarOrLet(c, assignments, symkind) + for resDef in resSection: + addToVarSection(c, result, n, resDef) + else: + if tup.kind == tyTuple and def.kind in {nkPar, nkTupleConstr} and + a.len > 3: + # var a, b = (1, 2) + message(c.config, a.info, warnEachIdentIsTuple) - for j in 0.. 0: v.flags.incl(sfShadowed) + for j in 0.. 0: v.flags.incl(sfShadowed) + else: + let shadowed = findShadowedVar(c, v) + if shadowed != nil: + shadowed.flags.incl(sfShadowed) + if shadowed.kind == skResult and sfGenSym notin v.flags: + message(c.config, a.info, warnResultShadowed) if def.kind != nkEmpty: if sfThread in v.flags: localError(c.config, def.info, errThreadvarCannotInit) setVarType(c, v, typ) @@ -708,35 +753,26 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode = b.add copyTree(def) addToVarSection(c, result, n, b) v.ast = b - else: - if def.kind in {nkPar, nkTupleConstr}: v.ast = def[j] - # bug #7663, for 'nim check' this can be a non-tuple: - if tup.kind == tyTuple: setVarType(c, v, tup[j]) - else: v.typ = tup - b[j] = newSymNode(v) - if def.kind == nkEmpty: - let actualType = v.typ.skipTypes({tyGenericInst, tyAlias, - tyUserTypeClassInst}) - if actualType.kind in {tyObject, tyDistinct} and - actualType.requiresInit: - defaultConstructionError(c, v.typ, v.info) - else: - checkNilable(c, v) - # allow let to not be initialised if imported from C: - if v.kind == skLet and sfImportc notin v.flags and (strictDefs notin c.features or not isLocalSym(v)): - localError(c.config, a.info, errLetNeedsInit) - if sfCompileTime in v.flags: - if a.kind != nkVarTuple: + if def.kind == nkEmpty: + let actualType = v.typ.skipTypes({tyGenericInst, tyAlias, + tyUserTypeClassInst}) + if actualType.kind in {tyObject, tyDistinct} and + actualType.requiresInit: + defaultConstructionError(c, v.typ, v.info) + else: + checkNilable(c, v) + # allow let to not be initialised if imported from C: + if v.kind == skLet and sfImportc notin v.flags and (strictDefs notin c.features or not isLocalSym(v)): + localError(c.config, a.info, errLetNeedsInit) + if sfCompileTime in v.flags: var x = newNodeI(result.kind, v.info) x.add result[i] vm.setupCompileTimeVar(c.module, c.idgen, c.graph, x) - else: - localError(c.config, a.info, "cannot destructure to compile time variable") - if v.flags * {sfGlobal, sfThread} == {sfGlobal}: - message(c.config, v.info, hintGlobalVar) - if {sfGlobal, sfPure} <= v.flags: - globalVarInitCheck(c, def) - suggestSym(c.graph, v.info, v, c.graph.usageSym) + if v.flags * {sfGlobal, sfThread} == {sfGlobal}: + message(c.config, v.info, hintGlobalVar) + if {sfGlobal, sfPure} <= v.flags: + globalVarInitCheck(c, def) + suggestSym(c.graph, v.info, v, c.graph.usageSym) proc semConst(c: PContext, n: PNode): PNode = result = copyNode(n) @@ -789,23 +825,19 @@ proc semConst(c: PContext, n: PNode): PNode = typeAllowedCheck(c, a.info, typ, skConst, typFlags) if a.kind == nkVarTuple: - if typ.kind != tyTuple: - localError(c.config, a.info, errXExpected, "tuple") - elif a.len-2 != typ.len: - localError(c.config, a.info, errWrongNumberOfVariables) - b = newNodeI(nkVarTuple, a.info) - newSons(b, a.len) - b[^2] = a[^2] - b[^1] = def + # generate new section from tuple unpacking and embed it into this one + let assignments = makeVarTupleSection(c, n, a, def, typ, skConst, result) + let resSection = semConst(c, assignments) + for resDef in resSection: + addToVarSection(c, result, n, resDef) + else: + for j in 0..} stmt typeDef = identVisDot genericParamList? pragma '=' optInd typeDefValue indAndComment? -varTuple = '(' optInd identWithPragma ^+ comma optPar ')' '=' optInd expr +varTupleLhs = '(' optInd (identWithPragma / varTupleLhs) ^+ comma optPar ')' +varTuple = varTupleLhs '=' optInd expr colonBody = colcom stmt postExprBlocks? variable = (varTuple / identColonEquals) colonBody? indAndComment constant = (varTuple / identWithPragma) (colon typeDesc)? '=' optInd expr indAndComment diff --git a/doc/manual.md b/doc/manual.md index 98f9d2ec3e..1fd35e7b3b 100644 --- a/doc/manual.md +++ b/doc/manual.md @@ -3080,8 +3080,8 @@ value is expected to come from native code, typically a C/C++ `const`. Tuple unpacking --------------- -In a `var` or `let` statement tuple unpacking can be performed. The special -identifier `_` can be used to ignore some parts of the tuple: +In a `var`, `let` or `const` statement tuple unpacking can be performed. +The special identifier `_` can be used to ignore some parts of the tuple: ```nim proc returnsTuple(): (int, int, int) = (4, 2, 3) @@ -3089,6 +3089,35 @@ identifier `_` can be used to ignore some parts of the tuple: let (x, _, z) = returnsTuple() ``` +This is treated as syntax sugar for roughly the following: + + ```nim + let + tmpTuple = returnsTuple() + x = tmpTuple[0] + z = tmpTuple[2] + ``` + +For `var` or `let` statements, if the value expression is a tuple literal, +each expression is directly expanded into an assignment without the use of +a temporary variable. + + ```nim + let (x, y, z) = (1, 2, 3) + # becomes + let + x = 1 + y = 2 + z = 3 + ``` + +Tuple unpacking can also be nested: + + ```nim + proc returnsNestedTuple(): (int, (int, int), int, int) = (4, (5, 7), 2, 3) + + let (x, (_, y), _, z) = returnsNestedTuple() + ``` Const section diff --git a/tests/arc/t19364.nim b/tests/arc/t19364.nim new file mode 100644 index 0000000000..f520f32916 --- /dev/null +++ b/tests/arc/t19364.nim @@ -0,0 +1,30 @@ +discard """ + cmd: '''nim c --gc:arc --expandArc:fooLeaks $file''' + nimout: ''' +--expandArc: fooLeaks + +var + tmpTuple_cursor + a_cursor + b_cursor + c_cursor +tmpTuple_cursor = refTuple +a_cursor = tmpTuple_cursor[0] +b_cursor = tmpTuple_cursor[1] +c_cursor = tmpTuple_cursor[2] +-- end of expandArc ------------------------ +''' +""" + +func fooLeaks(refTuple: tuple[a, + b, + c: seq[float]]): float = + let (a, b, c) = refTuple + +let refset = (a: newSeq[float](25_000_000), + b: newSeq[float](25_000_000), + c: newSeq[float](25_000_000)) + +var res = newSeq[float](1_000_000) +for i in 0 .. res.high: + res[i] = fooLeaks(refset) diff --git a/tests/macros/tastrepr.nim b/tests/macros/tastrepr.nim index 759ca55b5e..c04498a25b 100644 --- a/tests/macros/tastrepr.nim +++ b/tests/macros/tastrepr.nim @@ -8,7 +8,9 @@ for i, d in pairs(data): discard for i, (x, y) in pairs(data): discard -var (a, b) = (1, 2) +var + a = 1 + b = 2 var data = @[(1, "one"), (2, "two")] for (i, d) in pairs(data): diff --git a/tests/misc/t11634.nim b/tests/misc/t11634.nim index 4ecb6a53c5..390af40f4f 100644 --- a/tests/misc/t11634.nim +++ b/tests/misc/t11634.nim @@ -1,8 +1,5 @@ discard """ action: reject - nimout: ''' -t11634.nim(20, 7) Error: cannot destructure to compile time variable -''' """ type Foo = ref object diff --git a/tests/parser/ttupleunpack.nim b/tests/parser/ttupleunpack.nim index c7ab9ea153..860ef66cff 100644 --- a/tests/parser/ttupleunpack.nim +++ b/tests/parser/ttupleunpack.nim @@ -27,3 +27,51 @@ proc main() = main() main2() + +block: # nested unpacking + block: # simple let + let (a, (b, c), d) = (1, (2, 3), 4) + doAssert (a, b, c, d) == (1, 2, 3, 4) + let foo = (a, (b, c), d) + let (a2, (b2, c2), d2) = foo + doAssert (a, b, c, d) == (a2, b2, c2, d2) + + block: # var and assignment + var (x, (y, z), t) = ('a', (true, @[123]), "abc") + doAssert (x, y, z, t) == ('a', true, @[123], "abc") + (x, (y, z), t) = ('b', (false, @[456]), "def") + doAssert (x, y, z, t) == ('b', false, @[456], "def") + + block: # very nested + let (_, (_, (_, (_, (_, a))))) = (1, (2, (3, (4, (5, 6))))) + doAssert a == 6 + + block: # const + const (a, (b, c), d) = (1, (2, 3), 4) + doAssert (a, b, c, d) == (1, 2, 3, 4) + const foo = (a, (b, c), d) + const (a2, (b2, c2), d2) = foo + doAssert (a, b, c, d) == (a2, b2, c2, d2) + + block: # evaluation semantics preserved between literal and not literal + var s: seq[string] + block: # literal + let (a, (b, c), d) = ((s.add("a"); 1), ((s.add("b"); 2), (s.add("c"); 3)), (s.add("d"); 4)) + doAssert (a, b, c, d) == (1, 2, 3, 4) + doAssert s == @["a", "b", "c", "d"] + block: # underscore + s = @[] + let (a, (_, c), _) = ((s.add("a"); 1), ((s.add("b"); 2), (s.add("c"); 3)), (s.add("d"); 4)) + doAssert (a, c) == (1, 3) + doAssert s == @["a", "b", "c", "d"] + block: # temp + s = @[] + let foo = ((s.add("a"); 1), ((s.add("b"); 2), (s.add("c"); 3)), (s.add("d"); 4)) + let (a, (b, c), d) = foo + doAssert (a, b, c, d) == (1, 2, 3, 4) + doAssert s == @["a", "b", "c", "d"] + +block: # unary assignment unpacking + var a: int + (a,) = (1,) + doAssert a == 1 diff --git a/tests/tuples/mnimsconstunpack.nim b/tests/tuples/mnimsconstunpack.nim new file mode 100644 index 0000000000..65fafc12f8 --- /dev/null +++ b/tests/tuples/mnimsconstunpack.nim @@ -0,0 +1,4 @@ +proc foo(): tuple[a, b: string] = + result = ("a", "b") + +const (a, b*) = foo() diff --git a/tests/tuples/tnimsconstunpack.nim b/tests/tuples/tnimsconstunpack.nim new file mode 100644 index 0000000000..7860fc0a45 --- /dev/null +++ b/tests/tuples/tnimsconstunpack.nim @@ -0,0 +1,8 @@ +discard """ + action: compile + cmd: "nim e $file" +""" + +import mnimsconstunpack + +doAssert b == "b"