Skip to content

Commit

Permalink
sem: assign consistent elif/else node kinds (#925)
Browse files Browse the repository at this point in the history
## 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`
  • Loading branch information
zerbina authored Sep 27, 2023
1 parent 40616ab commit 014a2de
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 0 deletions.
3 changes: 3 additions & 0 deletions compiler/sem/semfold.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions compiler/sem/semstmts.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
19 changes: 19 additions & 0 deletions tests/lang_exprs/tif_stmt_regression.nim
Original file line number Diff line number Diff line change
@@ -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
)

0 comments on commit 014a2de

Please sign in to comment.