diff --git a/changelog.md b/changelog.md index c8a9c39c5d..0d1ca4ae02 100644 --- a/changelog.md +++ b/changelog.md @@ -33,6 +33,8 @@ 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. + ## Standard library additions and changes [//]: # "Additions:" diff --git a/compiler/lineinfos.nim b/compiler/lineinfos.nim index 397d407077..2d55e40667 100644 --- a/compiler/lineinfos.nim +++ b/compiler/lineinfos.nim @@ -97,6 +97,7 @@ type warnUnknownNotes = "UnknownNotes" warnUser = "User", warnGlobalVarConstructorTemporary = "GlobalVarConstructorTemporary", + warnImplicitRangeConversion = "ImplicitRangeConversion", # hints hintSuccess = "Success", hintSuccessX = "SuccessX", hintCC = "CC", @@ -204,6 +205,7 @@ const warnUnknownNotes: "$1", warnUser: "$1", warnGlobalVarConstructorTemporary: "global variable '$1' initialization requires a temporary variable", + warnImplicitRangeConversion: "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", @@ -258,7 +260,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} + result[3] = {low(TNoteKind)..high(TNoteKind)} - {warnObservableStores, warnResultUsed, warnAnyEnumConv, warnBareExcept, warnStdPrefix, warnImplicitRangeConversion} result[2] = result[3] - {hintStackTrace, hintExtendedContext, hintDeclaredLoc, hintProcessingStmt} result[1] = result[2] - {warnProveField, warnProveIndex, warnGcUnsafe, hintPath, hintDependency, hintCodeBegin, hintCodeEnd, diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim index 145d9ed103..31b4b5c1e2 100644 --- a/compiler/sigmatch.nim +++ b/compiler/sigmatch.nim @@ -615,6 +615,34 @@ proc isGenericObjectOf(f, a: PType): bool = # use sym equality to check if the `tyGenericBody` types are equal result = aRoot != nil and f.sym == aRoot.sym +proc isRangeSupertype(conf: ConfigRef; wider, narrower: PType): bool = + ## Check if `wider` type fully contains `narrower` type + ## Returns true if narrower fits entirely within wider (safe conversion) + if wider.isOrdinalType: + let wideFirst = firstOrd(conf, wider) + let wideLast = lastOrd(conf, wider) + let narrowFirst = firstOrd(conf, narrower) + let narrowLast = lastOrd(conf, narrower) + result = narrowFirst >= wideFirst and narrowLast <= wideLast + else: + let wideFirst = firstFloat(wider) + let wideLast = lastFloat(wider) + let narrowFirst = firstFloat(narrower) + let narrowLast = lastFloat(narrower) + result = narrowFirst >= wideFirst and narrowLast <= wideLast + +proc shouldWarnRangeConversion(conf: ConfigRef; 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) + else: + result = false + proc isObjectSubtype(c: var TCandidate; a, f, fGenericOrigin: PType): int = var t = a assert t.kind == tyObject @@ -2487,6 +2515,10 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType, case r of isConvertible: + # Check for problematic implicit range conversions + if shouldWarnRangeConversion(c.config, f, a): + message(c.config, arg.info, warnImplicitRangeConversion, + typeToString(a) & " -> " & typeToString(f)) if f.skipTypes({tyRange}).kind in {tyInt, tyUInt}: inc(m.convMatches) inc(m.convMatches) @@ -2507,6 +2539,10 @@ proc paramTypesMatchAux(m: var TCandidate, f, a: PType, of isIntConv: # I'm too lazy to introduce another ``*matches`` field, so we conflate # ``isIntConv`` and ``isIntLit`` here: + # Check for problematic implicit range conversions + if shouldWarnRangeConversion(c.config, f, a): + message(c.config, arg.info, warnImplicitRangeConversion, + typeToString(a) & " -> " & typeToString(f)) if f.skipTypes({tyRange}).kind notin {tyInt, tyUInt}: inc(m.intConvMatches) inc(m.intConvMatches) diff --git a/tests/range/timplicitrangedownsizing.nim b/tests/range/timplicitrangedownsizing.nim new file mode 100644 index 0000000000..1b10f2f322 --- /dev/null +++ b/tests/range/timplicitrangedownsizing.nim @@ -0,0 +1,73 @@ +discard """ +cmd: "nim check $options --hints:off --warning:ImplicitRangeConversion --warningaserror:ImplicitRangeConversion $file" +action: "reject" +nimout: ''' +timplicitrangedownsizing.nim(22, 5) Error: implicit range conversion int -> FakeUint8 [ImplicitRangeConversion] +timplicitrangedownsizing.nim(24, 5) Error: implicit range conversion OffByOneRange -> FakeUint8 [ImplicitRangeConversion] +timplicitrangedownsizing.nim(28, 5) Error: implicit range conversion int -> FakeUint8 [ImplicitRangeConversion] +timplicitrangedownsizing.nim(55, 6) Error: implicit range conversion float64 -> SmallFloat [ImplicitRangeConversion] +timplicitrangedownsizing.nim(59, 6) Error: implicit range conversion FloatRange -> SmallFloat [ImplicitRangeConversion] +timplicitrangedownsizing.nim(63, 6) Error: implicit range conversion float64 -> SmallFloat [ImplicitRangeConversion] +''' +""" +# Integer range tests +type FakeUint8 = range[0..255] +type OffByOneRange = range[1..256] +type WideRange = range[0..65535] + +var v: FakeUint8 +var x = 256 +var y = OffByOneRange(256) + +v = x # panics, should trigger warning +v = FakeUint8(x) # panics, should not trigger warning +v = y # panics should trigger warning + +proc xxx(v: FakeUint8)= discard + +xxx(x) # panics, should trigger warning +xxx(FakeUint8(x)) # panics, should not trigger warning + +# Test narrower to wider range conversions (should NOT warn) +proc acceptWide(v: WideRange) = discard + +var smallRange: FakeUint8 = FakeUint8(100) +acceptWide(smallRange) # OK - FakeUint8 (0..255) fits in WideRange (0..65535) + +var medRange: OffByOneRange = OffByOneRange(150) +acceptWide(medRange) # OK - OffByOneRange (1..256) fits in WideRange (0..65535) + +var w: WideRange +w = smallRange # OK - FakeUint8 range fits in WideRange +w = medRange # OK - OffByOneRange range fits in WideRange + +# Test narrower range passed to function (should NOT warn) +xxx(smallRange) # OK - FakeUint8 value fits in range[0..255] + +# Float range tests +type SmallFloat = range[0.0..10.0] +type FloatRange = range[5.0..15.0] +type WideFloatRange = range[0.0..100.0] + +var fv: SmallFloat +var fx = 11.5 # Out of range + +fv = fx # panics, should trigger warning +fv = SmallFloat(fx) # panics, should not trigger warning + +var fy = FloatRange(7.5) +fv = fy # panics, should trigger warning (5.0..15.0 → 0.0..10.0) + +proc fffx(v: SmallFloat) = discard + +fffx(fx) # panics, should trigger warning +fffx(SmallFloat(fx)) # panics, should not trigger warning + +# Test narrower to wider float range conversions (should NOT warn) +proc acceptWideFloat(v: WideFloatRange) = discard + +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 \ No newline at end of file