Skip to content

Commit

Permalink
semfold: fix range-check folding (#822)
Browse files Browse the repository at this point in the history
## Summary

Fix literals resulting from folding range-check conversions having a
type that doesn't match the node kind. This, in turn, fixes a VM code
generator issue where the code generated for an access of the result of
an integer-literal-converted-to-float-range would cause the VM to crash.

## Details

The folding logic directly assigned the destination type to the literal,
which led to, for example, integer literals having a `tyFloat` type.
This breaks the assumption of only float-literal nodes having a float
type, and is what caused the `vmgen` bug (`vmgen` only looks at the node
kind).

Same as for normal conversion, range-check conversion are now also
folded via `foldConv` (but only in case the range check succeeds),
fixing the issue.
  • Loading branch information
zerbina authored Jul 31, 2023
1 parent f02934a commit b794757
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
4 changes: 2 additions & 2 deletions compiler/sem/semfold.nim
Original file line number Diff line number Diff line change
Expand Up @@ -761,8 +761,8 @@ proc getConstExpr(m: PSym, n: PNode; idgen: IdGenerator; g: ModuleGraph): PNode
let a = getConstExpr(m, n[0], idgen, g)
if a == nil: return
if leValueConv(n[1], a) and leValueConv(a, n[2]):
result = a # a <= x and x <= b
result.typ = n.typ
# a <= x and x <= b
result = foldConv(n, a, idgen, g, check=false)
else:
result = g.config.newError(n,
PAstDiag(kind: adSemInvalidRangeConversion))
Expand Down
9 changes: 9 additions & 0 deletions tests/lang_types/range/trange.nim
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
discard """
targets: "c js vm"
output: '''
TSubRange: 5 from 1 to 10
#FF3722
Expand Down Expand Up @@ -154,3 +155,11 @@ block:
c = d
doAssert a == b
doAssert c == d

block int_literal_to_float_range:
type Range = range[0'f..1'f]

var val = 0'f
# use the value in a comparison in order to make sure that
# the conversion really worked
doAssert val == Range(0)

0 comments on commit b794757

Please sign in to comment.