From ccf51094dfe86713e5a50958a15d305a902f042a Mon Sep 17 00:00:00 2001 From: metagn Date: Mon, 4 Nov 2024 18:25:13 +0300 Subject: [PATCH 1/4] disallow implicit return if `result` is used, don't use void context --- compiler/semexprs.nim | 12 +++++++++++- compiler/semstmts.nim | 1 - tests/discard/tvoidcontext.nim | 12 ------------ tests/discard/tvoidcontext1.nim | 8 ++++++++ tests/discard/tvoidcontext2.nim | 9 +++++++++ tests/discard/twrongvoidcontext.nim | 10 ++++++++++ 6 files changed, 38 insertions(+), 14 deletions(-) delete mode 100644 tests/discard/tvoidcontext.nim create mode 100644 tests/discard/tvoidcontext1.nim create mode 100644 tests/discard/tvoidcontext2.nim create mode 100644 tests/discard/twrongvoidcontext.nim diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 6cc4afd6e0080..be2a5f1f86717 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2061,7 +2061,6 @@ proc semAsgn(c: PContext, n: PNode; mode=asgnNormal): PNode = let lhs = n[0] let rhs = semExprWithType(c, n[1], {efTypeAllowed}, le) if lhs.kind == nkSym and lhs.sym.kind == skResult: - n.typ() = c.enforceVoidContext if c.p.owner.kind != skMacro and resultTypeIsInferrable(lhs.sym.typ): var rhsTyp = rhs.typ if rhsTyp.kind in tyUserTypeClasses and rhsTyp.isResolvedUserTypeClass: @@ -2092,10 +2091,12 @@ proc semReturn(c: PContext, n: PNode): PNode = if c.p.owner.kind in {skConverter, skMethod, skProc, skFunc, skMacro} or (not c.p.owner.typ.isNil and isClosureIterator(c.p.owner.typ)): if n[0].kind != nkEmpty: + var removeResultSymUsed = false if n[0].kind == nkAsgn and n[0][0].kind == nkSym and c.p.resultSym == n[0][0].sym: discard "return is already transformed" elif c.p.resultSym != nil: # transform ``return expr`` to ``result = expr; return`` + removeResultSymUsed = sfUsed notin c.p.resultSym.flags var a = newNodeI(nkAsgn, n[0].info) a.add newSymNode(c.p.resultSym) a.add n[0] @@ -2104,6 +2105,8 @@ proc semReturn(c: PContext, n: PNode): PNode = localError(c.config, n.info, errNoReturnTypeDeclared) return result[0] = semAsgn(c, n[0]) + if removeResultSymUsed: + c.p.resultSym.flags.excl sfUsed # optimize away ``result = result``: if result[0][1].kind == nkSym and result[0][1].sym == c.p.resultSym: result[0] = c.graph.emptyNode @@ -2128,6 +2131,13 @@ proc semProcBody(c: PContext, n: PNode; expectedType: PType = nil): PNode = # are not expressions: fixNilType(c, result) else: + if sfUsed in c.p.resultSym.flags: + var last = result + while last.kind in {nkStmtList, nkStmtListExpr}: + last = last.lastSon + localError(c.config, last.info, "cannot use implicit return, " & + "the `result` symbol was used in '" & c.p.owner.name.s & "'") + incl(c.p.resultSym.flags, sfUsed) var a = newNodeI(nkAsgn, n.info, 2) a[0] = newSymNode(c.p.resultSym) a[1] = result diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index 60778d25d687b..d5b5a9c589567 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -1897,7 +1897,6 @@ proc addResult(c: PContext, n: PNode, t: PType, owner: TSymKind) = var s = newSym(skResult, getIdent(c.cache, "result"), c.idgen, getCurrOwner(c), n.info) s.typ = t - incl(s.flags, sfUsed) if owner == skMacro or t != nil: if n.len > resultPos and n[resultPos] != nil: diff --git a/tests/discard/tvoidcontext.nim b/tests/discard/tvoidcontext.nim deleted file mode 100644 index 9e2b913ad8812..0000000000000 --- a/tests/discard/tvoidcontext.nim +++ /dev/null @@ -1,12 +0,0 @@ -discard """ - errormsg: '''expression '"invalid"' is of type 'string' and has to be used (or discarded)''' - line: 12 -""" - -proc valid*(): string = - let x = 317 - "valid" - -proc invalid*(): string = - result = "foo" - "invalid" diff --git a/tests/discard/tvoidcontext1.nim b/tests/discard/tvoidcontext1.nim new file mode 100644 index 0000000000000..0d7319911e2c9 --- /dev/null +++ b/tests/discard/tvoidcontext1.nim @@ -0,0 +1,8 @@ +proc valid*(): string = + let x = 317 + "valid" + +proc invalid*(): string = + result = "foo" + "invalid" #[tt.Error + ^ cannot use implicit return, the `result` symbol was used in 'invalid']# diff --git a/tests/discard/tvoidcontext2.nim b/tests/discard/tvoidcontext2.nim new file mode 100644 index 0000000000000..50cdb8f3610ad --- /dev/null +++ b/tests/discard/tvoidcontext2.nim @@ -0,0 +1,9 @@ +proc foo(x: var string) = + x = "hello" + +proc bar(): string = + foo(result) + "invalid" #[tt.Error + ^ cannot use implicit return, the `result` symbol was used in 'bar']# + +echo bar() diff --git a/tests/discard/twrongvoidcontext.nim b/tests/discard/twrongvoidcontext.nim new file mode 100644 index 0000000000000..ed041934c02e5 --- /dev/null +++ b/tests/discard/twrongvoidcontext.nim @@ -0,0 +1,10 @@ +discard """ + output: ''' +true +''' +""" + +proc f(): int = + echo (result = 1; result > 0) + +doAssert f() == 1 From 8e5bcb05064d4ae3118c1d48ddeb2d1737ad8f35 Mon Sep 17 00:00:00 2001 From: metagn Date: Mon, 4 Nov 2024 18:25:57 +0300 Subject: [PATCH 2/4] add issue number --- tests/discard/twrongvoidcontext.nim | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/discard/twrongvoidcontext.nim b/tests/discard/twrongvoidcontext.nim index ed041934c02e5..b873d3d1ffc03 100644 --- a/tests/discard/twrongvoidcontext.nim +++ b/tests/discard/twrongvoidcontext.nim @@ -4,6 +4,8 @@ true ''' """ +# issue #16855 + proc f(): int = echo (result = 1; result > 0) From 9d5c443817fa01d03e28e35f432e7f1ef6024867 Mon Sep 17 00:00:00 2001 From: metagn Date: Mon, 4 Nov 2024 18:31:08 +0300 Subject: [PATCH 3/4] include type in error message --- compiler/semexprs.nim | 3 ++- tests/discard/tvoidcontext1.nim | 2 +- tests/discard/tvoidcontext2.nim | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index be2a5f1f86717..7a695f6238433 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2136,7 +2136,8 @@ proc semProcBody(c: PContext, n: PNode; expectedType: PType = nil): PNode = while last.kind in {nkStmtList, nkStmtListExpr}: last = last.lastSon localError(c.config, last.info, "cannot use implicit return, " & - "the `result` symbol was used in '" & c.p.owner.name.s & "'") + "the `result` symbol was used in '" & c.p.owner.name.s & "'; " & + "got expression of type '" & last.typ.typeToString & "'") incl(c.p.resultSym.flags, sfUsed) var a = newNodeI(nkAsgn, n.info, 2) a[0] = newSymNode(c.p.resultSym) diff --git a/tests/discard/tvoidcontext1.nim b/tests/discard/tvoidcontext1.nim index 0d7319911e2c9..f6964bfe5b1f4 100644 --- a/tests/discard/tvoidcontext1.nim +++ b/tests/discard/tvoidcontext1.nim @@ -5,4 +5,4 @@ proc valid*(): string = proc invalid*(): string = result = "foo" "invalid" #[tt.Error - ^ cannot use implicit return, the `result` symbol was used in 'invalid']# + ^ cannot use implicit return, the `result` symbol was used in 'invalid'; got expression of type 'string']# diff --git a/tests/discard/tvoidcontext2.nim b/tests/discard/tvoidcontext2.nim index 50cdb8f3610ad..9f1df828d89ca 100644 --- a/tests/discard/tvoidcontext2.nim +++ b/tests/discard/tvoidcontext2.nim @@ -4,6 +4,6 @@ proc foo(x: var string) = proc bar(): string = foo(result) "invalid" #[tt.Error - ^ cannot use implicit return, the `result` symbol was used in 'bar']# + ^ cannot use implicit return, the `result` symbol was used in 'bar'; got expression of type 'string']# echo bar() From 46206d6ce0762017e7b046a65747e2ef0231d1b3 Mon Sep 17 00:00:00 2001 From: metagn Date: Mon, 4 Nov 2024 19:19:08 +0300 Subject: [PATCH 4/4] completely consider void, giving discard check error again --- compiler/semexprs.nim | 13 ++++--------- lib/packages/docutils/rst.nim | 2 +- tests/discard/tvoidcontext1.nim | 8 ++++++-- tests/discard/tvoidcontext2.nim | 8 ++++++-- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 7a695f6238433..445ab908fcb3c 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2119,7 +2119,10 @@ proc semProcBody(c: PContext, n: PNode; expectedType: PType = nil): PNode = return n openScope(c) result = semExpr(c, n, expectedType = expectedType) - if c.p.resultSym != nil and not isEmptyType(result.typ): + if c.p.resultSym != nil and + not isEmptyType(result.typ) and + sfUsed notin c.p.resultSym.flags: + incl(c.p.resultSym.flags, sfUsed) if result.kind == nkNilLit: # or ImplicitlyDiscardable(result): # new semantic: 'result = x' triggers the void context @@ -2131,14 +2134,6 @@ proc semProcBody(c: PContext, n: PNode; expectedType: PType = nil): PNode = # are not expressions: fixNilType(c, result) else: - if sfUsed in c.p.resultSym.flags: - var last = result - while last.kind in {nkStmtList, nkStmtListExpr}: - last = last.lastSon - localError(c.config, last.info, "cannot use implicit return, " & - "the `result` symbol was used in '" & c.p.owner.name.s & "'; " & - "got expression of type '" & last.typ.typeToString & "'") - incl(c.p.resultSym.flags, sfUsed) var a = newNodeI(nkAsgn, n.info, 2) a[0] = newSymNode(c.p.resultSym) a[1] = result diff --git a/lib/packages/docutils/rst.nim b/lib/packages/docutils/rst.nim index 4d2ce3e5f612a..3502d52774ab5 100644 --- a/lib/packages/docutils/rst.nim +++ b/lib/packages/docutils/rst.nim @@ -2842,7 +2842,7 @@ proc readTableRow(p: var RstParser): ColSeq = proc getColContents(p: var RstParser, colLim: ColumnLimits): string = for i in colLim.first ..< colLim.last: result.add(p.tok[i].symbol) - result.strip + result = result.strip proc isValidDelimiterRow(p: var RstParser, colNum: int): bool = let row = readTableRow(p) diff --git a/tests/discard/tvoidcontext1.nim b/tests/discard/tvoidcontext1.nim index f6964bfe5b1f4..9e2b913ad8812 100644 --- a/tests/discard/tvoidcontext1.nim +++ b/tests/discard/tvoidcontext1.nim @@ -1,8 +1,12 @@ +discard """ + errormsg: '''expression '"invalid"' is of type 'string' and has to be used (or discarded)''' + line: 12 +""" + proc valid*(): string = let x = 317 "valid" proc invalid*(): string = result = "foo" - "invalid" #[tt.Error - ^ cannot use implicit return, the `result` symbol was used in 'invalid'; got expression of type 'string']# + "invalid" diff --git a/tests/discard/tvoidcontext2.nim b/tests/discard/tvoidcontext2.nim index 9f1df828d89ca..a1f7c5eb9b8a3 100644 --- a/tests/discard/tvoidcontext2.nim +++ b/tests/discard/tvoidcontext2.nim @@ -1,9 +1,13 @@ +discard """ + errormsg: '''expression '"invalid"' is of type 'string' and has to be used (or discarded)''' + line: 11 +""" + proc foo(x: var string) = x = "hello" proc bar(): string = foo(result) - "invalid" #[tt.Error - ^ cannot use implicit return, the `result` symbol was used in 'bar'; got expression of type 'string']# + "invalid" echo bar()