Skip to content

Commit

Permalink
semfold: fix conversion folding (#823)
Browse files Browse the repository at this point in the history
## Summary

Make the logic handling folding of integer- and float-like conversion
more exhaustive and slightly simplify it. This fixes the internal issue
of literal-nodes having unexpected types, and it also fixes no range
error being reported at compile-time for some to-`char` conversion.

## Details

Folding conversions to integer, float, and bool types now considers all
valid source types, with unexpected types now being treated as a defect
instead of silently ignoring them. Named set constants are used for
this, with `tyChar` being included in the integer-like set, meaning that
to-`char` conversions are now properly subjected to range checks during
constant folding.

For the integer-like branch handling, the logic is simplified to only
perform the range check in a single place, and the unnecessary
transition-to-`nkUIntLit` removed (`newIntNodeType` already makes sure
that the node has the correct kind based on the type).

In general, this is a step towards the type of a literal-node matching
its kind. Both the C and VM code generator use the node kind for
deciding whether a float literal is a 32-bit or 64-bit one, so the
float-literal-nodes produced by constant folding now having the correct
kind fixes a bug where the emitted float literals had the wrong target-
language type (see the fixed test `tfloats.nim`).
  • Loading branch information
zerbina authored Aug 2, 2023
1 parent 50c2ac0 commit 89adfe1
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 34 deletions.
53 changes: 23 additions & 30 deletions compiler/sem/semfold.nim
Original file line number Diff line number Diff line change
Expand Up @@ -439,51 +439,44 @@ proc foldConv(n, a: PNode; idgen: IdGenerator; g: ModuleGraph; check = false): P
PAstDiag(kind: adSemFoldRangeCheckForLiteralConversionFailed,
inputLit: a))

# if srcTyp.kind == tyUInt64 and "FFFFFF" in $n:
# echo "n: ", n, " a: ", a
# echo "from: ", srcTyp, " to: ", dstTyp, " check: ", check
# echo getInt(a)
# echo high(int64)
# writeStackTrace()
const
IntegerLike = {tyInt..tyInt64, tyUInt..tyUInt64, tyChar}
FloatLike = {tyFloat..tyFloat64}

case dstTyp.kind
of tyBool:
case srcTyp.kind
of tyFloat..tyFloat64:
of FloatLike:
result = newIntNodeT(toInt128(getFloat(a) != 0.0), n, idgen, g)
of tyChar, tyUInt..tyUInt64, tyInt..tyInt64:
of IntegerLike:
result = newIntNodeT(toInt128(a.getOrdValue != 0), n, idgen, g)
of tyBool, tyEnum: # xxx shouldn't we disallow `tyEnum`?
result = a
result.typ = n.typ
else: doAssert false, $srcTyp.kind
of tyInt..tyInt64, tyUInt..tyUInt64:
case srcTyp.kind
of tyFloat..tyFloat64:
result = newIntNodeT(toInt128(getFloat(a)), n, idgen, g)
of tyChar, tyUInt..tyUInt64, tyInt..tyInt64:
let val = a.getOrdValue
if check and not rangeCheck(n, val, g):
result = rangeError(n, a, g)
else:
result = newIntNodeT(val, n, idgen, g)
if dstTyp.kind in {tyUInt..tyUInt64}:
result.transitionIntKind(nkUIntLit)
else: unreachable(srcTyp.kind)
of IntegerLike:
let val =
case srcTyp.kind
of IntegerLike, tyEnum, tyBool: getOrdValue(a)
of FloatLike: toInt128(getFloat(a))
else: unreachable(srcTyp.kind)

if check and not rangeCheck(n, val, g):
result = rangeError(n, a, g)
else:
result = a
result.typ = n.typ
if check and result.kind in {nkCharLit..nkUInt64Lit} and
not rangeCheck(n, getInt(result), g):
result = rangeError(n, a, g)
of tyFloat..tyFloat64:
result = newIntNodeT(val, n, idgen, g)
of FloatLike:
case srcTyp.kind
of tyInt..tyInt64, tyEnum, tyBool, tyChar:
of IntegerLike, tyEnum, tyBool:
result = newFloatNodeT(toFloat64(getOrdValue(a)), n, g)
of FloatLike:
result = newFloatNodeT(a.floatVal, n, g)
else:
result = a
result.typ = n.typ
unreachable(srcTyp.kind)
of tyOpenArray, tyVarargs, tyProc, tyPointer:
discard
else:
# FIXME: conversion-to-enum is missing checks
result = a
result.typ = n.typ

Expand Down
5 changes: 1 addition & 4 deletions tests/lang_types/float/tfloats.nim
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,7 @@ template main =
doAssert $(x0, x1, x2, x3) == "(1.32, 1.32, 1.32, 1.32)"
block: # example https://github.com/nim-lang/Nim/issues/12884#issuecomment-564967962
let x = float(1.32'f32)
when nimvm: discard # xxx prints 1.3
else:
when not defined(js):
doAssert $x == "1.3200000524520874"
doAssert $x == "1.32"
doAssert $1.32 == "1.32"
doAssert $1.32'f32 == "1.32"
let x2 = 1.32'f32
Expand Down
3 changes: 3 additions & 0 deletions tests/misc/tconv.nim
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ template accept(x) =
reject:
const x = int8(300)

reject:
const x = char(high(int32))

reject:
const x = int64(NaN)

Expand Down

0 comments on commit 89adfe1

Please sign in to comment.