Skip to content

Commit

Permalink
Fix endsInNoReturn for case statements (#23009)
Browse files Browse the repository at this point in the history
While looking at the CI I noticed that there's a couple false positives
for `case` statements that cannot be checked for exhaustiveness since my
changes, this should resolve them.

---------

Co-authored-by: SirOlaf <>
  • Loading branch information
SirOlaf authored Nov 30, 2023
1 parent 0f7ebb4 commit 9140f8e
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 14 deletions.
23 changes: 21 additions & 2 deletions compiler/sem.nim
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,18 @@ proc commonType*(c: PContext; x, y: PType): PType =
result = newType(k, c.idgen, r.owner)
result.addSonSkipIntLit(r, c.idgen)

const shouldChckCovered = {tyInt..tyInt64, tyChar, tyEnum, tyUInt..tyUInt64, tyBool}
proc shouldCheckCaseCovered(caseTyp: PType): bool =
result = false
case caseTyp.kind
of shouldChckCovered:
result = true
of tyRange:
if skipTypes(caseTyp[0], abstractInst).kind in shouldChckCovered:
result = true
else:
discard

proc endsInNoReturn(n: PNode): bool =
## check if expr ends the block like raising or call of noreturn procs do
result = false # assume it does return
Expand Down Expand Up @@ -239,6 +251,12 @@ proc endsInNoReturn(n: PNode): bool =
# none of the branches returned
result = hasElse # Only truly a no-return when it's exhaustive
of nkCaseStmt:
let caseTyp = skipTypes(it[0].typ, abstractVar-{tyTypeDesc})
# semCase should already have checked for exhaustiveness in this case
# effectively the same as having an else
var hasElse = caseTyp.shouldCheckCaseCovered()

# actual noreturn checks
for i in 1 ..< it.len:
let branch = it[i]
checkBranch:
Expand All @@ -248,11 +266,12 @@ proc endsInNoReturn(n: PNode): bool =
of nkElifBranch:
branch[1]
of nkElse:
hasElse = true
branch[0]
else:
raiseAssert "Malformed `case` statement in endsInNoReturn"
# none of the branches returned
result = true
# Can only guarantee a noreturn if there is an else or it's exhaustive
result = hasElse
of nkTryStmt:
checkBranch(it[0])
for i in 1 ..< it.len:
Expand Down
10 changes: 2 additions & 8 deletions compiler/semstmts.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1146,20 +1146,14 @@ proc semCase(c: PContext, n: PNode; flags: TExprFlags; expectedType: PType = nil
openScope(c)
pushCaseContext(c, n)
n[0] = semExprWithType(c, n[0])
var chckCovered = false
var covered: Int128 = toInt128(0)
var typ = commonTypeBegin
var expectedType = expectedType
var hasElse = false
let caseTyp = skipTypes(n[0].typ, abstractVar-{tyTypeDesc})
const shouldChckCovered = {tyInt..tyInt64, tyChar, tyEnum, tyUInt..tyUInt64, tyBool}
var chckCovered = caseTyp.shouldCheckCaseCovered()
case caseTyp.kind
of shouldChckCovered:
chckCovered = true
of tyRange:
if skipTypes(caseTyp[0], abstractInst).kind in shouldChckCovered:
chckCovered = true
of tyFloat..tyFloat128, tyString, tyCstring, tyError:
of tyFloat..tyFloat128, tyString, tyCstring, tyError, shouldChckCovered, tyRange:
discard
else:
popCaseContext(c)
Expand Down
45 changes: 41 additions & 4 deletions tests/controlflow/tunreachable.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ discard """
cmd: "nim check --warningAsError:UnreachableCode $file"
action: "reject"
nimout: '''
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]
tunreachable.nim(26, 3) Error: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
tunreachable.nim(33, 3) Error: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
tunreachable.nim(42, 3) Error: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
tunreachable.nim(65, 5) Error: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
tunreachable.nim(77, 5) Error: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
'''
"""

Expand Down Expand Up @@ -39,4 +41,39 @@ proc main3() =
return
echo "after"

main3()
main3()


block:
# Cases like strings are not checked for exhaustiveness unless they have an else
proc main4(x: string) =
case x
of "a":
return
# reachable
echo "after"

main4("a")

proc main5(x: string) =
case x
of "a":
return
else:
return
# unreachable
echo "after"

main5("a")

block:
# In this case no else is needed because it's exhaustive
proc exhaustive(x: bool) =
case x
of true:
return
of false:
return
echo "after"

exhaustive(true)

0 comments on commit 9140f8e

Please sign in to comment.