Skip to content

Commit

Permalink
fix(sem): avoid no-return false positives (#1452)
Browse files Browse the repository at this point in the history
## Summary
no-return analysis now no longer confuses  `void`  branches for true
no-returns.

## Details

No-return analysis now dives into the trees of  `if/try/block/case` 
statements in order to determine if they are in fact no-return
scenarios. This is only done for the statement variety.

The updated analysis besides being more correct, in that it avoids false
positive no-return identification, it still allows for a number of false
negatives. Chiefly that unstructured exits ( `return/break/raise` ) are
only considered in the trailing position of statement lists and not part
way through. This is considered acceptable as there is an overall
improvement in accuracy and it should cover most code.

This includes a fix for an NPE crash in discard-or-use error reporting.
  • Loading branch information
saem authored Sep 7, 2024
1 parent 4feef40 commit 961aaa7
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 6 deletions.
22 changes: 18 additions & 4 deletions compiler/ast/ast_query.nim
Original file line number Diff line number Diff line change
Expand Up @@ -695,11 +695,25 @@ proc endsInNoReturn*(n: PNode): bool =
## etc.) or a call of a noreturn proc. This is meant to be called on a
## semmed `n`.
var it = n
while it.kind in {nkStmtList, nkStmtListExpr} and it.len > 0:
it = it.lastSon
while it.kind in {nkStmtList, nkStmtListExpr} and it.len > 0 or
it.kind in {nkIfStmt, nkCaseStmt, nkBlockStmt, nkTryStmt} and it.typ.isEmptyType:
case it.kind
of nkStmtList, nkStmtListExpr, nkBlockStmt:
it = it.lastSon
of nkIfStmt, nkCaseStmt:
it = it.lastSon.lastSon
of nkTryStmt:
it =
case it[^1].kind
of nkFinally:
it[^2]
of nkExceptBranch:
it[^1]
of nkAllNodeKinds - {nkFinally, nkExceptBranch}:
unreachable()
else:
unreachable()
result = it.kind in nkLastBlockStmts or
(it.kind in {nkIfStmt, nkTryStmt, nkCaseStmt, nkBlockStmt} and
it.typ.isEmptyType()) or
it.kind in nkCallKinds and it[0].kind == nkSym and sfNoReturn in it[0].sym.flags

type
Expand Down
6 changes: 4 additions & 2 deletions compiler/front/cli_reporter.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1216,13 +1216,15 @@ proc reportBody*(conf: ConfigRef, r: SemReport): string =
result = "illegal discard"

of rsemUseOrDiscardExpr:
let n = r.wrongNode
let
n = r.wrongNode
typStr = if n.typ.isNil: "void" else: n.typ.skipTypes({tyVar}).render

result.add(
"expression '",
n.render,
"' is of type '",
n.typ.skipTypes({tyVar}).render,
typStr,
"' and has to be used (or discarded)"
)

Expand Down
16 changes: 16 additions & 0 deletions tests/lang_exprs/tnoreturn_nested_reject.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
discard """
description: '''
Ensure nested non-noreturn statements aren't treated as such
'''
errormsg: "expression '10' is of type 'int literal(10)' and has to be used (or discarded)"
line: 11
"""

let x =
if true:
10
else:
try:
discard
except:
discard

0 comments on commit 961aaa7

Please sign in to comment.