From 014a2de69310d801fb58e7ab70c5d6e56f5e4c21 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Wed, 27 Sep 2023 14:29:39 +0200
Subject: [PATCH] sem: assign consistent `elif`/`else` node kinds (#925)

## Summary

Always assign the node kind for `elif`/`else` AST such that whether the
expression/statement version is used is consistent with the  `if`  AST.
In
other words, a (valid) `nkIfStmt` node now only has `nkElifBranch`/
`nkElse` nodes as immediate sub-nodes, while a `nkIfExpr` node only has
`nkElifExpr`/`nkElseExpr` nodes as immediate sub-nodes.

This affects `typed` AST, and it also fixes an internal compiler error
with  `if`  statements within statement-list expressions, which was
caused
by the node kind discrepancy.

## Details

* in `semIf`, transition branch nodes to their statement/expression
  version, if necessary. This makes whether the statement/expression
  version is used is consistent with `nkIfExpr`/`nkIfStmt`
* this fixes `semfold.foldInAst` (the constant-folding pass) skipping
  if-statement-within-statement-list-expressions. For these,
  `nkElifExpr` and `nkElseExpr` nodes appeared as sub-nodes to
  `nkIfStmt` AST, which lead to the do-nothing catch-all being taken
  (and folding thus being skipped)
* until the catch-all branch in `foldInAst` is removed, and in
  order to prevent silent regressions, both `nkElifExpr` and `nkElse`
  are now explicitly disallowed (`unreachable`) in `foldInAst`
---
 compiler/sem/semfold.nim                 |  3 +++
 compiler/sem/semstmts.nim                | 12 ++++++++++++
 tests/lang_exprs/tif_stmt_regression.nim | 19 +++++++++++++++++++
 3 files changed, 34 insertions(+)
 create mode 100644 tests/lang_exprs/tif_stmt_regression.nim

diff --git a/compiler/sem/semfold.nim b/compiler/sem/semfold.nim
index a2b6e2f0d21..a2bf81793f0 100644
--- a/compiler/sem/semfold.nim
+++ b/compiler/sem/semfold.nim
@@ -961,6 +961,9 @@ proc foldInAstAux(m: PSym, n: PNode, idgen: IdGenerator, g: ModuleGraph): Folded
     # don't process the symbol slots
     result = fromAst(n, n.len - 1)
     result.add foldInAstAux(m, n[^1], idgen, g)
+  of nkElifExpr, nkElseExpr:
+    # must not exist in statement AST
+    unreachable(n.kind)
   else:
     # XXX: type AST reaches here, but ideally this catch-all branch
     #      wouldn't be needed
diff --git a/compiler/sem/semstmts.nim b/compiler/sem/semstmts.nim
index 58a720f0457..cbf453950d4 100644
--- a/compiler/sem/semstmts.nim
+++ b/compiler/sem/semstmts.nim
@@ -173,6 +173,12 @@ proc semIf(c: PContext, n: PNode; flags: TExprFlags): PNode =
   elif isEmptyType(typ) or typ.kind in {tyNil, tyUntyped} or
       (not hasElse and efInTypeof notin flags):
     for it in n:
+      # transition the branch to its statement variant:
+      case it.kind
+      of nkElifExpr: it.transitionSonsKind(nkElifBranch)
+      of nkElseExpr: it.transitionSonsKind(nkElse)
+      else:          discard "already correct"
+
       it[^1] = discardCheck(c, it[^1], flags)
       if it[^1].isError:
         return wrapError(c.config, result)
@@ -181,6 +187,12 @@ proc semIf(c: PContext, n: PNode; flags: TExprFlags): PNode =
     if typ == c.enforceVoidContext: result.typ = c.enforceVoidContext
   else:
     for it in n:
+      # transition the branch to its expression variant:
+      case it.kind
+      of nkElifBranch: it.transitionSonsKind(nkElifExpr)
+      of nkElse:       it.transitionSonsKind(nkElseExpr)
+      else:            discard "already correct"
+
       let j = it.len-1
       if not endsInNoReturn(it[j]):
         it[j] = fitNode(c, typ, it[j], it[j].info)
diff --git a/tests/lang_exprs/tif_stmt_regression.nim b/tests/lang_exprs/tif_stmt_regression.nim
new file mode 100644
index 00000000000..c14689179b7
--- /dev/null
+++ b/tests/lang_exprs/tif_stmt_regression.nim
@@ -0,0 +1,19 @@
+discard """
+  description: '''
+    Regression test for ``if`` statements within statement-list expressions.
+    They were not being processed by constant folding
+  '''
+"""
+
+var x = (
+  # unfolded 'true'/'false' symbols lead to an internal compiler error,
+  # which is used for detecting whether folding took place here
+  if true:
+    # branch content doesn't matter, as long as it's a statement
+    var y = false
+  elif true: # also test an elif branch
+    var y = false
+  else:
+    var y = false
+  1
+)
\ No newline at end of file