mirror of
https://github.com/nim-lang/Nim.git
synced 2026-05-24 21:59:52 +00:00
fix string setLenUninit growth without realloc for refc (#25767)
`setLenUninit(string)` was broken on the legacy refc backend when
growing within existing spare capacity.
`setLengthStrUninit` in `lib/system/sysstr.nim` only updated len when it
had to reallocate or when shrinking.
If oldLen < newLen <= capacity, it returned early without finalizing:
```nim
var s = newStringOfCap(10)
s.add("abc")
s.setLenUninit(6)
doAssert s.len == 6 # used to fail, len stayed 3
```
This escaped `tests/stdlib/tstring.nim` because the testing routine
`checkSetLenUninit` mostly resizes strings created at **exact**
length/capacity, so growth usually took the reallocating branch.
The new regression test covers the missing edge case.
So sorry for catching this only on the day of the stable release! In my
defense, the original PR hung in limbo for quite a while and it didn't
spend enough time in devel after the merge.
This commit is contained in:
@@ -264,7 +264,7 @@ proc setLengthStrUninit(s: var string, newlen: Natural) {.nodestroy.} =
|
||||
str.data[n] = '\0'
|
||||
str.len = n
|
||||
s = cast[string](str)
|
||||
elif n < s.len:
|
||||
elif n != s.len:
|
||||
str.data[n] = '\0'
|
||||
str.len = n
|
||||
else: return
|
||||
|
||||
@@ -176,6 +176,13 @@ proc main() =
|
||||
result.add char(ord('a') + i mod 26)
|
||||
|
||||
proc checkSetLenUninit(oldLen, newLen: int; cmpAfter = -1) =
|
||||
## Verifies `setLenUninit`:
|
||||
## - preserves the existing prefix
|
||||
## - updates the string length
|
||||
## - keeps internal null termination valid for both shrink and growth
|
||||
##
|
||||
## `cmpAfter` is used for layouts where trailing zeroed padding affects
|
||||
## string comparison semantics after the resize.
|
||||
var s = makeStr(oldLen)
|
||||
let prefixLen = min(oldLen, newLen)
|
||||
let prefix = makeStr(prefixLen)
|
||||
@@ -203,9 +210,18 @@ proc main() =
|
||||
|
||||
block setLenUninit:
|
||||
# Shared baseline for both SSO and V2: noop, shrink, grow.
|
||||
checkSetLenUninit(numbers.len, numbers.len)
|
||||
checkSetLenUninit(numbers.len, 5)
|
||||
checkSetLenUninit(numbers.len, 11)
|
||||
checkSetLenUninit(10, 10)
|
||||
checkSetLenUninit(10, 5)
|
||||
checkSetLenUninit(10, 11)
|
||||
|
||||
block growingWithinBiggerCapacity:
|
||||
# Strings can reserve spare capacity even for short strings.
|
||||
# Growing within that capacity must still update len and the trailing zero.
|
||||
var s = newStringOfCap(10)
|
||||
s.add("abc")
|
||||
s.setLenUninit(6)
|
||||
s.checkStrInternals(6)
|
||||
doAssert s[0..2] == "abc"
|
||||
|
||||
when hasNativeSso:
|
||||
const
|
||||
|
||||
Reference in New Issue
Block a user