Make range checks in semConv (#7164)

* Remove NaN/Inf/NegInf magic
* Make range checks in semConv
* fix the failing line
* fix `firstOrd` and `lastOrd`
* fix `localError`
* remove debug comment
* Cleanup, fix failing test
* make tests green

changelog.md

@@ -34,6 +34,7 @@
 
 - To use multi-methods, explicit `--multimethods:on` is now needed.
 
+- Compile time checks for integer and float conversions are now stricter. For example, `const x = uint32(-1)` now gives a compile time error instead of being equivalent to `const x = 0xFFFFFFFF'u32`.
 
 #### Breaking changes in the standard library
 

compiler/ast.nim

@@ -656,7 +656,6 @@ type
     mVoidType, mPNimrodNode, mShared, mGuarded, mLock, mSpawn, mDeepCopy,
     mIsMainModule, mCompileDate, mCompileTime, mProcCall,
     mCpuEndian, mHostOS, mHostCPU, mBuildOS, mBuildCPU, mAppType,
-    mNaN, mInf, mNegInf,
     mCompileOption, mCompileOptionArg,
     mNLen, mNChild, mNSetChild, mNAdd, mNAddMultiple, mNDel,
     mNKind, mNSymKind,

compiler/semexprs.nim

@@ -99,7 +99,8 @@ type
   TConvStatus = enum
     convOK,
     convNotNeedeed,
-    convNotLegal
+    convNotLegal,
+    convNotInRange
 
 proc checkConversionBetweenObjects(castDest, src: PType; pointers: int): TConvStatus =
   let diff = inheritanceDiff(castDest, src)
@@ -111,18 +112,16 @@ proc checkConversionBetweenObjects(castDest, src: PType; pointers: int): TConvSt
 const
   IntegralTypes = {tyBool, tyEnum, tyChar, tyInt..tyUInt64}
 
-proc checkConvertible(c: PContext, castDest, src: PType): TConvStatus =
+proc checkConvertible(c: PContext, targetTyp: PType, src: PNode): TConvStatus =
+  let srcTyp = src.typ.skipTypes({tyStatic})
   result = convOK
-  # We're interested in the inner type and not in the static tag
-  var src = src.skipTypes({tyStatic})
-  if sameType(castDest, src) and castDest.sym == src.sym:
+  if sameType(targetTyp, srcTyp) and targetTyp.sym == srcTyp.sym:
     # don't annoy conversions that may be needed on another processor:
-    if castDest.kind notin IntegralTypes+{tyRange}:
+    if targetTyp.kind notin IntegralTypes+{tyRange}:
       result = convNotNeedeed
     return
-  # Save for later
-  var d = skipTypes(castDest, abstractVar)
-  var s = src
+  var d = skipTypes(targetTyp, abstractVar)
+  var s = srcTyp
   if s.kind in tyUserTypeClasses and s.isResolvedUserTypeClass:
     s = s.lastSon
   s = skipTypes(s, abstractVar-{tyTypeDesc, tyOwned})
@@ -138,19 +137,36 @@ proc checkConvertible(c: PContext, castDest, src: PType): TConvStatus =
       d = d.lastSon
       s = s.lastSon
     inc pointers
+
+  let targetBaseTyp = skipTypes(targetTyp, abstractVarRange)
+  let srcBaseTyp = skipTypes(srcTyp, abstractVarRange-{tyTypeDesc})
+
   if d == nil:
     result = convNotLegal
   elif d.kind == tyObject and s.kind == tyObject:
     result = checkConversionBetweenObjects(d, s, pointers)
-  elif (skipTypes(castDest, abstractVarRange).kind in IntegralTypes) and
-      (skipTypes(src, abstractVarRange-{tyTypeDesc}).kind in IntegralTypes):
-    # accept conversion between integral types
-    discard
+  elif (targetBaseTyp.kind in IntegralTypes) and
+      (srcBaseTyp.kind in IntegralTypes):
+    if targetTyp.isOrdinalType:
+      if src.kind in nkCharLit..nkUInt64Lit and
+          src.intVal notin firstOrd(c.config, targetTyp)..lastOrd(c.config, targetTyp):
+        result = convNotInRange
+      elif src.kind in nkFloatLit..nkFloat64Lit and
+          (classify(src.floatVal) in {fcNaN, fcNegInf, fcInf} or
+            src.floatVal.int64 notin firstOrd(c.config, targetTyp)..lastOrd(c.config, targetTyp)):
+        result = convNotInRange
+    elif targetBaseTyp.kind in tyFloat..tyFloat64:
+      if src.kind in nkFloatLit..nkFloat64Lit and
+          not floatRangeCheck(src.floatVal, targetTyp):
+        result = convNotInRange
+      elif src.kind in nkCharLit..nkUInt64Lit and
+          not floatRangeCheck(src.intval.float, targetTyp):
+        result = convNotInRange
   else:
     # we use d, s here to speed up that operation a bit:
     case cmpTypes(c, d, s)
     of isNone, isGeneric:
-      if not compareTypes(castDest.skipTypes(abstractVar), src.skipTypes({tyOwned}), dcEqIgnoreDistinct):
+      if not compareTypes(targetTyp.skipTypes(abstractVar), srcTyp.skipTypes({tyOwned}), dcEqIgnoreDistinct):
         result = convNotLegal
     else:
       discard
@@ -260,7 +276,7 @@ proc semConv(c: PContext, n: PNode): PNode =
   addSon(result, op)
 
   if not isSymChoice(op):
-    let status = checkConvertible(c, result.typ, op.typ)
+    let status = checkConvertible(c, result.typ, op)
     case status
     of convOK:
       # handle SomeProcType(SomeGenericProc)
@@ -275,10 +291,15 @@ proc semConv(c: PContext, n: PNode): PNode =
       if result == nil:
         localError(c.config, n.info, "illegal conversion from '$1' to '$2'" %
           [op.typ.typeToString, result.typ.typeToString])
+    of convNotInRange:
+      let value =
+        if op.kind in {nkCharLit..nkUInt64Lit}: $op.getInt else: $op.getFloat
+      localError(c.config, n.info, errGenerated, value & " can't be converted to " &
+        result.typ.typeToString)
   else:
     for i in 0 ..< sonsLen(op):
       let it = op.sons[i]
-      let status = checkConvertible(c, result.typ, it.typ)
+      let status = checkConvertible(c, result.typ, it)
       if status in {convOK, convNotNeedeed}:
         markUsed(c.config, n.info, it.sym, c.graph.usageSym)
         onUse(n.info, it.sym)

compiler/semfold.nim

@@ -559,9 +559,6 @@ proc getConstExpr(m: PSym, n: PNode; g: ModuleGraph): PNode =
       of mBuildOS: result = newStrNodeT(toLowerAscii(platform.OS[g.config.target.hostOS].name), n, g)
       of mBuildCPU: result = newStrNodeT(platform.CPU[g.config.target.hostCPU].name.toLowerAscii, n, g)
       of mAppType: result = getAppType(n, g)
-      of mNaN: result = newFloatNodeT(NaN, n, g)
-      of mInf: result = newFloatNodeT(Inf, n, g)
-      of mNegInf: result = newFloatNodeT(NegInf, n, g)
       of mIntDefine:
         if isDefined(g.config, s.name.s):
           try:
@@ -733,7 +730,7 @@ proc getConstExpr(m: PSym, n: PNode; g: ModuleGraph): PNode =
     var a = getConstExpr(m, n.sons[1], g)
     if a == nil: return
     # XXX: we should enable `check` for other conversion types too
-    result = foldConv(n, a, g, check=n.kind == nkHiddenSubConv)
+    result = foldConv(n, a, g, check=n.kind == nkHiddenStdConv)
   of nkCast:
     var a = getConstExpr(m, n.sons[1], g)
     if a == nil: return

compiler/types.nim

@@ -741,6 +741,22 @@ proc lastFloat*(t: PType): BiggestFloat =
     internalError(newPartialConfigRef(), "invalid kind for lastFloat(" & $t.kind & ')')
     NaN
 
+proc floatRangeCheck*(x: BiggestFloat, t: PType): bool =
+  case t.kind
+  # This needs to be special cased since NaN is never
+  # part of firstFloat(t) .. lastFloat(t)
+  of tyFloat..tyFloat128:
+    true
+  of tyRange:
+    x in firstFloat(t) .. lastFloat(t)
+  of tyVar:
+    floatRangeCheck(x, t.sons[0])
+  of tyGenericInst, tyDistinct, tyTypeDesc, tyAlias, tySink,
+     tyStatic, tyInferred, tyUserTypeClasses:
+    floatRangeCheck(x, lastSon(t))
+  else:
+    internalError(newPartialConfigRef(), "invalid kind for floatRangeCheck:" & $t.kind)
+    false
 
 proc lengthOrd*(conf: ConfigRef; t: PType): BiggestInt =
   case t.skipTypes(tyUserTypeClasses).kind

lib/posix/posix_nintendoswitch_consts.nim

@@ -243,7 +243,7 @@ const IPPROTO_TCP* = cint(6)
 const IPPROTO_UDP* = cint(17)
 const INADDR_ANY* = InAddrScalar(0)
 const INADDR_LOOPBACK* = InAddrScalar(2130706433)
-const INADDR_BROADCAST* = InAddrScalar(-1)
+const INADDR_BROADCAST* = InAddrScalar(4294967295)
 const INET_ADDRSTRLEN* = cint(16)
 const INET6_ADDRSTRLEN* = cint(46)
 const IPV6_JOIN_GROUP* = cint(12)

lib/pure/segfaults.nim

@@ -25,7 +25,7 @@ when defined(windows):
   import winlean
 
   const
-    EXCEPTION_ACCESS_VIOLATION = DWORD(0xc0000005)
+    EXCEPTION_ACCESS_VIOLATION = DWORD(0xc0000005'i32)
     EXCEPTION_CONTINUE_SEARCH = Long(0)
 
   type

lib/windows/winlean.nim

@@ -832,9 +832,9 @@ template hasOverlappedIoCompleted*(lpOverlapped): bool =
   (cast[uint](lpOverlapped.internal) != STATUS_PENDING)
 
 const
- IOC_OUT* = 0x40000000
- IOC_IN*  = 0x80000000
- IOC_WS2* = 0x08000000
+ IOC_OUT* = 0x40000000'i32
+ IOC_IN*  = 0x80000000'i32
+ IOC_WS2* = 0x08000000'i32
  IOC_INOUT* = IOC_IN or IOC_OUT
 
 template WSAIORW*(x,y): untyped = (IOC_INOUT or x or y)

tests/misc/tconv.nim

@@ -0,0 +1,43 @@
+template reject(x) =
+    static: assert(not compiles(x))
+
+reject:
+    const x = int8(300)
+
+reject:
+    const x = int64(NaN)
+
+type
+    R = range[0..10]
+
+reject:
+    const x = R(11)
+
+reject:
+    const x = R(11.0)
+
+reject:
+    const x = R(NaN)
+
+reject:
+    const x = R(Inf)
+
+type
+    FloatRange = range[0'f..10'f]
+
+reject:
+    const x = FloatRange(-1'f)
+
+reject:
+    const x = FloatRange(-1)
+
+reject:
+    const x = FloatRange(NaN)
+
+block:
+    const x = float32(NaN)
+
+type E = enum a, b, c
+
+reject:
+    const e = E(4)