diff --git a/changelog.md b/changelog.md index 8d59320672..665346ad70 100644 --- a/changelog.md +++ b/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 diff --git a/compiler/lineinfos.nim b/compiler/lineinfos.nim index 75a73549da..5a9b3d49b5 100644 --- a/compiler/lineinfos.nim +++ b/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, diff --git a/compiler/nim.cfg b/compiler/nim.cfg index 9dab29eeed..425f0df324 100644 --- a/compiler/nim.cfg +++ b/compiler/nim.cfg @@ -65,3 +65,7 @@ define:useStdoutAsStdmsg @if nimHasVtables: experimental:vtables @end + +@if nimHasImplicitRangeConversion: + warning[ImplicitRangeConversion]:off +@end diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 7bd339fea6..cf8dd88a19 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -167,7 +167,7 @@ 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}) @@ -175,7 +175,19 @@ proc shouldWarnRangeConversion(conf: ConfigRef; formalType, argType: PType): boo 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 @@ -1537,7 +1549,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)) diff --git a/doc/manual.md b/doc/manual.md index 85ce2dbe26..556fba538b 100644 --- a/doc/manual.md +++ b/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 -------------------------------- diff --git a/tests/range/timplicitrangedownsizing.nim b/tests/range/timplicitrangedownsizing.nim index 1b10f2f322..930d91bb11 100644 --- a/tests/range/timplicitrangedownsizing.nim +++ b/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 \ No newline at end of file +wf = smallFloatRange # OK - SmallFloat range fits in WideFloatRange + +proc foo(x: Natural) = + discard + +foo(12) \ No newline at end of file diff --git a/tests/range/timplicitrangedownsizing2.nim b/tests/range/timplicitrangedownsizing2.nim new file mode 100644 index 0000000000..679fad60be --- /dev/null +++ b/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) \ No newline at end of file