mirror of
https://github.com/nim-lang/Nim.git
synced 2026-02-12 06:18:51 +00:00
fixes #24706
(cherry picked from commit bfc2786718)
This commit is contained in:
@@ -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:"
|
||||
|
||||
@@ -174,3 +174,5 @@ proc initDefines*(symbols: StringTableRef) =
|
||||
|
||||
defineSymbol("nimHasPreviewDuplicateModuleError")
|
||||
defineSymbol("nimHasSetLengthSeqUninitMagic")
|
||||
|
||||
defineSymbol("nimHasImplicitRangeConversion")
|
||||
|
||||
@@ -98,6 +98,7 @@ type
|
||||
warnLongLiterals = "LongLiterals",
|
||||
warnUser = "User",
|
||||
warnGlobalVarConstructorTemporary = "GlobalVarConstructorTemporary",
|
||||
warnImplicitRangeConversion = "ImplicitRangeConversion",
|
||||
# hints
|
||||
hintSuccess = "Success", hintSuccessX = "SuccessX",
|
||||
hintCC = "CC",
|
||||
@@ -206,6 +207,7 @@ const
|
||||
warnLongLiterals: "$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",
|
||||
@@ -260,7 +262,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,
|
||||
|
||||
@@ -84,6 +84,7 @@ type
|
||||
gcUnsafe, isRecursive, isTopLevel, hasSideEffect, inEnforcedGcSafe: bool
|
||||
isInnerProc: bool
|
||||
inEnforcedNoSideEffects: bool
|
||||
isArrayIndexing: bool
|
||||
currentExceptType: PType
|
||||
unknownRaises: seq[(PSym, TLineInfo)]
|
||||
currOptions: TOptions
|
||||
@@ -147,6 +148,37 @@ proc isLocalSym(a: PEffects, s: PSym): bool =
|
||||
s.typ != nil and (s.kind in {skLet, skVar, skResult} or (s.kind == skParam and isOutParam(s.typ))) and
|
||||
sfGlobal notin s.flags and s.owner == a.owner
|
||||
|
||||
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
|
||||
elif not narrower.isOrdinalType:
|
||||
let wideFirst = firstFloat(wider)
|
||||
let wideLast = lastFloat(wider)
|
||||
let narrowFirst = firstFloat(narrower)
|
||||
let narrowLast = lastFloat(narrower)
|
||||
result = narrowFirst >= wideFirst and narrowLast <= wideLast
|
||||
else:
|
||||
# int -> float ranges; warn
|
||||
result = false
|
||||
|
||||
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 lockLocations(a: PEffects; pragma: PNode) =
|
||||
if pragma.kind != nkExprColonExpr:
|
||||
localError(a.config, pragma.info, "locks pragma without argument")
|
||||
@@ -1503,6 +1535,11 @@ proc track(tracked: PEffects, n: PNode) =
|
||||
message(tracked.config, n.info, warnPtrToCstringConv,
|
||||
$n[1].typ)
|
||||
|
||||
# Check for implicit range conversions
|
||||
if n.kind == nkHiddenStdConv and (not tracked.isArrayIndexing) and
|
||||
shouldWarnRangeConversion(tracked.config, n.typ, n[1].typ):
|
||||
message(tracked.config, n.info, warnImplicitRangeConversion,
|
||||
typeToString(n[1].typ) & " -> " & typeToString(n.typ))
|
||||
|
||||
let t = n.typ.skipTypes(abstractInst)
|
||||
if t.kind == tyEnum:
|
||||
@@ -1541,7 +1578,12 @@ proc track(tracked: PEffects, n: PNode) =
|
||||
checkBounds(tracked, n[0], n[1])
|
||||
track(tracked, n[0])
|
||||
dec tracked.leftPartOfAsgn
|
||||
for i in 1 ..< n.len: track(tracked, n[i])
|
||||
for i in 1 ..< n.len:
|
||||
if i == 1:
|
||||
tracked.isArrayIndexing = true
|
||||
track(tracked, n[i])
|
||||
if i == 1:
|
||||
tracked.isArrayIndexing = false
|
||||
inc tracked.leftPartOfAsgn
|
||||
of nkError:
|
||||
localError(tracked.config, n.info, errorToString(tracked.config, n))
|
||||
|
||||
@@ -617,6 +617,8 @@ 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 isObjectSubtype(c: var TCandidate; a, f, fGenericOrigin: PType): int =
|
||||
var t = a
|
||||
assert t.kind == tyObject
|
||||
|
||||
73
tests/range/timplicitrangedownsizing.nim
Normal file
73
tests/range/timplicitrangedownsizing.nim
Normal file
@@ -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
|
||||
Reference in New Issue
Block a user