From 3b206ed988765419c5ff02c2b08645f24ed0cbad Mon Sep 17 00:00:00 2001
From: SirOlaf <34164198+SirOlaf@users.noreply.github.com>
Date: Fri, 1 Sep 2023 06:41:39 +0200
Subject: [PATCH] Fix #22604: Make endsInNoReturn traverse the tree (#22612)

* 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 <>
---
 compiler/hlo.nim                   |  3 --
 compiler/sem.nim                   | 59 +++++++++++++++++++++++++++---
 compiler/semexprs.nim              |  1 -
 compiler/semstmts.nim              |  4 +-
 tests/controlflow/tunreachable.nim | 14 ++++++-
 tests/exprs/t22604.nim             | 49 +++++++++++++++++++++++++
 6 files changed, 116 insertions(+), 14 deletions(-)
 create mode 100644 tests/exprs/t22604.nim

diff --git a/compiler/hlo.nim b/compiler/hlo.nim
index 744fddcc0f63..9fdec38c0edb 100644
--- a/compiler/hlo.nim
+++ b/compiler/hlo.nim
@@ -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 =
diff --git a/compiler/sem.nim b/compiler/sem.nim
index 653d83aaa682..7a49def53f96 100644
--- a/compiler/sem.nim
+++ b/compiler/sem.nim
@@ -26,7 +26,10 @@ when not defined(leanCompiler):
   import spawn
 
 when defined(nimPreviewSlimSystem):
-  import std/formatfloat
+  import std/[
+    formatfloat,
+    assertions,
+  ]
 
 # implementation
 
@@ -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
diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index 65ed25015ee8..f612cd9968ee 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -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)
diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim
index f9168eef631d..ee1b56fed4cb 100644
--- a/compiler/semstmts.nim
+++ b/compiler/semstmts.nim
@@ -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
diff --git a/tests/controlflow/tunreachable.nim b/tests/controlflow/tunreachable.nim
index 11c8595ebf35..64e199e17f01 100644
--- a/tests/controlflow/tunreachable.nim
+++ b/tests/controlflow/tunreachable.nim
@@ -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]
 '''
 """
   
@@ -30,3 +31,12 @@ proc main2() =
   echo "after"
 
 main2()
+
+proc main3() =
+  if true:
+    return
+  else:
+    return
+  echo "after"
+
+main3()
\ No newline at end of file
diff --git a/tests/exprs/t22604.nim b/tests/exprs/t22604.nim
new file mode 100644
index 000000000000..570f989d64ae
--- /dev/null
+++ b/tests/exprs/t22604.nim
@@ -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"
\ No newline at end of file