From 4feef40ad25987f200e0fb9ae78462078bf5ab61 Mon Sep 17 00:00:00 2001
From: Saem Ghani <saemghani+github@gmail.com>
Date: Fri, 6 Sep 2024 18:50:59 -0700
Subject: [PATCH] fix(sem): analysis considers nested noreturn (#1450)

## Summary
No-return analysis ensures that nested final expressions (
`block/case/if/try` ) are accounted for no-return itself.

## Details
Prior to this change if the last expression within a greater expression
was a no-return expression it would not be deemed as such if it was a
`block/case/if/try` , instead errors would be raised related to discard
checks or that path not resulting in the correct type.

For example, the following would result in an error where  `0`  must be
used or discarded:
```nim
proc foo() =
  let x =
    if false: 10
    else:
      block:
        return
```

Fixes https://github.com/nim-works/nimskull/issues/1440

---------

Co-authored-by: zerbina <100542850+zerbina@users.noreply.github.com>
---
 compiler/ast/ast_query.nim            | 12 ++++++
 compiler/sem/sem.nim                  |  8 ----
 compiler/sem/semstmts.nim             |  1 -
 tests/lang_exprs/tnoreturn_nested.nim | 53 +++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 9 deletions(-)
 create mode 100644 tests/lang_exprs/tnoreturn_nested.nim

diff --git a/compiler/ast/ast_query.nim b/compiler/ast/ast_query.nim
index 7a5bed931c7..f9d22db016e 100644
--- a/compiler/ast/ast_query.nim
+++ b/compiler/ast/ast_query.nim
@@ -690,6 +690,18 @@ iterator genericParamsInMacroCall*(macroSym: PSym, call: PNode): (PSym, PNode) =
     if posInCall < call.len:
       yield (genericParam, call[posInCall])
 
+proc endsInNoReturn*(n: PNode): bool =
+  ## Checks if expression `n` ends in an unstructured exit (raise, return,
+  ## 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
+  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
   NodePosName* = enum
     ## Named node position accessor
diff --git a/compiler/sem/sem.nim b/compiler/sem/sem.nim
index f63228593ea..489e518f358 100644
--- a/compiler/sem/sem.nim
+++ b/compiler/sem/sem.nim
@@ -390,14 +390,6 @@ proc commonType*(c: PContext; x, y: PType): PType =
         result = newType(k, nextTypeId(c.idgen), r.owner)
         result.addSonSkipIntLit(r, c.idgen)
 
-proc endsInNoReturn(n: PNode): bool =
-  # check if expr ends in raise exception or call of noreturn proc
-  var it = n
-  while it.kind in {nkStmtList, nkStmtListExpr} 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
-
 proc commonType*(c: PContext; x: PType, y: PNode): PType =
   # ignore exception raising branches in case/if expressions
   addInNimDebugUtils(c.config, "commonType", y, x, result)
diff --git a/compiler/sem/semstmts.nim b/compiler/sem/semstmts.nim
index 9052ceeb6b7..2f51a410fce 100644
--- a/compiler/sem/semstmts.nim
+++ b/compiler/sem/semstmts.nim
@@ -1705,7 +1705,6 @@ proc semCase(c: PContext, n: PNode; flags: TExprFlags): PNode =
       localReport(c.config, result, SemReport(
         kind: rsemMissingCaseBranches,
         nodes: formatMissingBranches(c, result)))
-
     else:
       localReport(c.config, result, reportSem rsemMissingCaseBranches)
 
diff --git a/tests/lang_exprs/tnoreturn_nested.nim b/tests/lang_exprs/tnoreturn_nested.nim
new file mode 100644
index 00000000000..786ee54d31a
--- /dev/null
+++ b/tests/lang_exprs/tnoreturn_nested.nim
@@ -0,0 +1,53 @@
+discard """
+  description: '''
+    Ensure nested noreturn statements are considererd in noreturn analysis
+  '''
+"""
+
+block nested_in_if:
+  let x =
+    if true:
+      0
+    else:
+      if true:
+        raise newException(CatchableError, "error")
+      else:
+        raise newException(CatchableError, "another error")
+
+  doAssert x is int
+
+block nested_in_try:
+  let x =
+    if true:
+      0
+    else:
+      try:
+        raise newException(CatchableError, "error")
+      finally:
+        discard
+
+  doAssert x is int
+
+block nested_in_case:
+  let x =
+    if true:
+      0
+    else:
+      let s = false
+      case s
+      of true:
+        raise newException(CatchableError, "error")
+      of false:
+        raise newException(CatchableError, "another error")
+
+  doAssert x is int
+
+block nested_in_block:
+  let x =
+    if true:
+      0
+    else:
+      block:
+        raise newException(CatchableError, "error")
+
+  doAssert x is int
\ No newline at end of file