enable --warning:ImplicitRangeConversion

changelog.md

@@ -33,7 +33,7 @@ errors.
 
 - Bitshift operators (`shl`, `shr`, `ashr`) now apply bitmasking to the right operand in the C/C++/VM/JS backends.
 
-- Adds a new warning enabled by `--warning:ImplicitRangeConversion` that detects downsizing implicit conversions to range types (e.g., `int -> range[0..255]` or `range[1..256] -> range[0..255]`) that could cause runtime panics. Safe conversions like `range[0..255] -> range[0..65535]` and explicit casts are not warned on.
+- Adds a new warning `--warning:ImplicitRangeConversion` that detects downsizing implicit conversions to range types (e.g., `int -> range[0..255]` or `range[1..256] -> range[0..255]`) that could cause runtime panics. Safe conversions like `range[0..255] -> range[0..65535]` and explicit casts do not trigger warnings. `int` to `Natural` and `Positive` conversions do not trigger warnings, which can be enabled with `--warning:systemRangeConversion`.
 
 ## Standard library additions and changes
 

compiler/lineinfos.nim

@@ -99,6 +99,7 @@ type
     warnUser = "User",
     warnGlobalVarConstructorTemporary = "GlobalVarConstructorTemporary",
     warnImplicitRangeConversion = "ImplicitRangeConversion",
+    warnSystemRangeConversion = "SystemRangeConversion",
     # hints
     hintSuccess = "Success", hintSuccessX = "SuccessX",
     hintCC = "CC",
@@ -208,6 +209,7 @@ const
     warnUser: "$1",
     warnGlobalVarConstructorTemporary: "global variable '$1' initialization requires a temporary variable",
     warnImplicitRangeConversion: "implicit range conversion $1",
+    warnSystemRangeConversion: "implicit range conversion $1",
     hintSuccess: "operation successful: $#",
     # keep in sync with `testament.isSuccess`
     hintSuccessX: "$build\n$loc lines; ${sec}s; $mem; proj: $project; out: $output",
@@ -262,7 +264,7 @@ type
 
 proc computeNotesVerbosity(): array[0..3, TNoteKinds] =
   result = default(array[0..3, TNoteKinds])
-  result[3] = {low(TNoteKind)..high(TNoteKind)} - {warnObservableStores, warnResultUsed, warnAnyEnumConv, warnBareExcept, warnStdPrefix, warnImplicitRangeConversion}
+  result[3] = {low(TNoteKind)..high(TNoteKind)} - {warnObservableStores, warnResultUsed, warnAnyEnumConv, warnBareExcept, warnStdPrefix, warnSystemRangeConversion}
   result[2] = result[3] - {hintStackTrace, hintExtendedContext, hintDeclaredLoc, hintProcessingStmt}
   result[1] = result[2] - {warnProveField, warnProveIndex,
     warnGcUnsafe, hintPath, hintDependency, hintCodeBegin, hintCodeEnd,

compiler/nim.cfg

@@ -65,3 +65,7 @@ define:useStdoutAsStdmsg
 @if nimHasVtables:
   experimental:vtables
 @end
+
+@if nimHasImplicitRangeConversion:
+  warning[ImplicitRangeConversion]:off
+@end

compiler/sempass2.nim

@@ -168,15 +168,27 @@ proc isRangeSupertype(conf: ConfigRef; wider, narrower: PType): bool =
     # int -> float ranges; warn
     result = false
 
-proc shouldWarnRangeConversion(conf: ConfigRef; formalType, argType: PType): bool =
+proc shouldWarnRangeConversion(conf: ConfigRef; info: TLineInfo; formalType, argType: PType): bool =
   ## Determine if an implicit range conversion should warn
   ## We warn on conversions that are likely to cause panics
   let f = formalType.skipTypes({tyGenericInst, tyAlias, tySink, tyDistinct})
   let a = argType.skipTypes({tyGenericInst, tyAlias, tySink, tyDistinct})
   if f.kind == tyRange:
     # Only warn if formal range doesn't fully contain argument range
     # Check if the ranges don't perfectly overlap
-    result = not isRangeSupertype(conf, f, a)
+    if a.kind == tyInt and f.sym != nil and f.sym.owner != nil and
+        sfSystemModule in f.sym.owner.flags and
+        (f.sym.name.s == "Positive" or
+          f.sym.name.s == "Natural"):
+      # Positive and Natural are special cases that we do not warn on with
+      # ImplicitRangeConversion, but may warn on with systemRangeConversion
+      # if that warning is enabled.
+      if conf.hasWarn(warnSystemRangeConversion):
+        message(conf, info, warnSystemRangeConversion,
+              typeToString(argType) & " -> " & typeToString(formalType))
+      result = false
+    else:
+      result = not isRangeSupertype(conf, f, a)
   else:
     result = false
 
@@ -1538,7 +1550,7 @@ proc track(tracked: PEffects, n: PNode) =
 
     # Check for implicit range conversions
     if n.kind == nkHiddenStdConv and (not tracked.isArrayIndexing) and
-          shouldWarnRangeConversion(tracked.config, n.typ, n[1].typ):
+          shouldWarnRangeConversion(tracked.config, n.info, n.typ, n[1].typ):
       message(tracked.config, n.info, warnImplicitRangeConversion,
               typeToString(n[1].typ) & " -> " & typeToString(n.typ))
 

doc/manual.md

@@ -1144,6 +1144,8 @@ semantic analysis). Assignments from the base type to one of its subrange types
 A subrange type has the same size as its base type (`int` in the
 Subrange example).
 
+Implicit "downsizing" conversions to range types (for example, `int -> range[0..255]` or `range[1..256] -> range[0..255]`) emit the `ImplicitRangeConversion` warning. Conversions that are clearly safe (for example, `range[0..255] -> range[0..65535]`) and any explicit casts do not trigger this warning. Conversions from `int` to common subranges such as `Natural` or `Positive` do not trigger this warning by default, but can be enabled with `--warning:systemRangeConversion`.
+
 
 Pre-defined floating-point types
 --------------------------------

tests/range/timplicitrangedownsizing.nim

@@ -70,4 +70,9 @@ var smallFloatRange: SmallFloat = SmallFloat(5.0)
 acceptWideFloat(smallFloatRange)  # OK - SmallFloat (0.0..10.0) fits in WideFloatRange (0.0..100.0)
 
 var wf: WideFloatRange
-wf = smallFloatRange  # OK - SmallFloat range fits in WideFloatRange
+wf = smallFloatRange  # OK - SmallFloat range fits in WideFloatRange
+
+proc foo(x: Natural) =
+  discard
+
+foo(12)

tests/range/timplicitrangedownsizing2.nim

@@ -0,0 +1,11 @@
+discard """
+  matrix: "--warning:systemRangeConversion --warningaserror:systemRangeConversion"
+  action: "reject"
+  errormsg: "implicit range conversion int literal(12) -> Natural"
+"""
+
+
+proc foo(x: Natural) =
+  discard
+
+foo(12)