Skip to content

Commit

Permalink
Fix #22604: Make endsInNoReturn traverse the tree (#22612)
Browse files Browse the repository at this point in the history
* Rewrite endsInNoReturn

* Handle `try` stmt again and add tests

* Fix unreachable code warning

* Remove unreachable code in semexprs again

* Check `it.len` before skip

* Move import of assertions

---------

Co-authored-by: SirOlaf <>
  • Loading branch information
SirOlaf authored Sep 1, 2023
1 parent ba158d7 commit 3b206ed
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 14 deletions.
3 changes: 0 additions & 3 deletions compiler/hlo.nim
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@
# This include implements the high level optimization pass.
# included from sem.nim

when defined(nimPreviewSlimSystem):
import std/assertions

proc hlo(c: PContext, n: PNode): PNode

proc evalPattern(c: PContext, n, orig: PNode): PNode =
Expand Down
59 changes: 54 additions & 5 deletions compiler/sem.nim
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ when not defined(leanCompiler):
import spawn

when defined(nimPreviewSlimSystem):
import std/formatfloat
import std/[
formatfloat,
assertions,
]

# implementation

Expand Down Expand Up @@ -207,12 +210,58 @@ proc commonType*(c: PContext; x, y: PType): PType =
result.addSonSkipIntLit(r, c.idgen)

proc endsInNoReturn(n: PNode): bool =
# check if expr ends in raise exception or call of noreturn proc
## check if expr ends the block like raising or call of noreturn procs do
result = false # assume it does return

template checkBranch(branch) =
if not endsInNoReturn(branch):
# proved a branch returns
return false

var it = n
while it.kind in {nkStmtList, nkStmtListExpr} and it.len > 0:
# skip these beforehand, no special handling needed
while it.kind in {nkStmtList, nkStmtListExpr, nkBlockStmt} and it.len > 0:
it = it.lastSon
result = it.kind in nkLastBlockStmts or
it.kind in nkCallKinds and it[0].kind == nkSym and sfNoReturn in it[0].sym.flags

case it.kind
of nkIfStmt:
var hasElse = false
for branch in it:
checkBranch:
if branch.len == 2:
branch[1]
elif branch.len == 1:
hasElse = true
branch[0]
else:
raiseAssert "Malformed `if` statement during endsInNoReturn"
# none of the branches returned
result = hasElse # Only truly a no-return when it's exhaustive
of nkCaseStmt:
for i in 1 ..< it.len:
let branch = it[i]
checkBranch:
case branch.kind
of nkOfBranch:
branch[^1]
of nkElifBranch:
branch[1]
of nkElse:
branch[0]
else:
raiseAssert "Malformed `case` statement in endsInNoReturn"
# none of the branches returned
result = true
of nkTryStmt:
checkBranch(it[0])
for i in 1 ..< it.len:
let branch = it[i]
checkBranch(branch[^1])
# none of the branches returned
result = true
else:
result = it.kind in nkLastBlockStmts or
it.kind in nkCallKinds and it[0].kind == nkSym and sfNoReturn in it[0].sym.flags

proc commonType*(c: PContext; x: PType, y: PNode): PType =
# ignore exception raising branches in case/if expressions
Expand Down
1 change: 0 additions & 1 deletion compiler/semexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1113,7 +1113,6 @@ proc semIndirectOp(c: PContext, n: PNode, flags: TExprFlags; expectedType: PType
msg.addDeclaredLocMaybe(c.config, typ)
localError(c.config, n.info, msg)
return errorNode(c, n)
result = nil
else:
result = m.call
instGenericConvertersSons(c, result, m)
Expand Down
4 changes: 1 addition & 3 deletions compiler/semstmts.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2651,9 +2651,7 @@ proc semStmtList(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType =
var m = n[i]
while m.kind in {nkStmtListExpr, nkStmtList} and m.len > 0: # from templates
m = m.lastSon
if m.kind in nkLastBlockStmts or
m.kind in nkCallKinds and m[0].kind == nkSym and
sfNoReturn in m[0].sym.flags:
if endsInNoReturn(m):
for j in i + 1..<n.len:
case n[j].kind
of nkPragma, nkCommentStmt, nkNilLit, nkEmpty, nkState: discard
Expand Down
14 changes: 12 additions & 2 deletions tests/controlflow/tunreachable.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ discard """
cmd: "nim check --warningAsError:UnreachableCode $file"
action: "reject"
nimout: '''
tunreachable.nim(23, 3) Error: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
tunreachable.nim(30, 3) Error: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
tunreachable.nim(24, 3) Error: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
tunreachable.nim(31, 3) Error: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
tunreachable.nim(40, 3) Error: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
'''
"""

Expand All @@ -30,3 +31,12 @@ proc main2() =
echo "after"

main2()

proc main3() =
if true:
return
else:
return
echo "after"

main3()
49 changes: 49 additions & 0 deletions tests/exprs/t22604.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# if
for i in 0..<1:
let x =
case false
of true:
42
of false:
if true:
continue
else:
raiseAssert "Won't get here"

# block
for i in 0..<1:
let x =
case false
of true:
42
of false:
block:
if true:
continue
else:
raiseAssert "Won't get here"

# nested case
for i in 0..<1:
let x =
case false
of true:
42
of false:
case true
of true:
continue
of false:
raiseAssert "Won't get here"

# try except
for i in 0..<1:
let x =
case false
of true:
42
of false:
try:
continue
except:
raiseAssert "Won't get here"

0 comments on commit 3b206ed

Please sign in to comment.