Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix endsInNoReturn for case statements #23009

Merged
merged 3 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
Loading