From 393d27b57da69bd864f456b878ff682a49202b86 Mon Sep 17 00:00:00 2001 From: Rybnikov Alex Date: Thu, 21 May 2026 06:40:35 -0500 Subject: [PATCH] fix(stdlib): use first-element flag in `$` for collections (#18583) (#25832) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #18583. ## Problem Several stdlib collection types compute the separator for `$` using `result.len > 1`, where `result` starts as the opening bracket (`"["` or `"{"`). This breaks when a collection element type has a `$` that returns an empty string: `result.len` stays at 1 after the first item contributes nothing, so the separator is never inserted for subsequent items. ```nim import std/deques type Test = object proc `$`(x: Test): string = "" echo [Test(), Test()].toDeque # prints [] — expected [, ] ``` ## Fix Replace the length check with an explicit `first` flag in all affected modules: `deques`, `heapqueue`, `lists`, `critbits`, and `strtabs`. ## Tests Regression tests added to `tdeques`, `theapqueue`, and `tlists` using a local type whose `$` returns `""`. All three test files pass with `nim c -r`. ## Notes I work with Claude as a co-processor. I'm 56, came to programming late, and this is genuinely how I learn and contribute. I understand what I'm submitting, but I didn't write it alone. If your project prefers human-only contributions, just say so and I'll close without friction. --------- Co-authored-by: Claude Sonnet 4.6 --- lib/pure/collections/critbits.nim | 7 +++++-- lib/pure/collections/deques.nim | 4 +++- lib/pure/collections/heapqueue.nim | 4 +++- lib/pure/collections/lists.nim | 4 +++- lib/pure/strtabs.nim | 4 +++- tests/stdlib/tdeques.nim | 9 +++++++++ tests/stdlib/theapqueue.nim | 12 ++++++++++++ tests/stdlib/tlists.nim | 11 +++++++++++ 8 files changed, 49 insertions(+), 6 deletions(-) diff --git a/lib/pure/collections/critbits.nim b/lib/pure/collections/critbits.nim index 6435abd0d0..c8ce7a4d20 100644 --- a/lib/pure/collections/critbits.nim +++ b/lib/pure/collections/critbits.nim @@ -495,13 +495,16 @@ func `$`*[T](c: CritBitTree[T]): string = const avgItemLen = 16 result = newStringOfCap(c.count * avgItemLen) result.add("{") + var first = true when T is void: for key in keys(c): - if result.len > 1: result.add(", ") + if first: first = false + else: result.add(", ") result.addQuoted(key) else: for key, val in pairs(c): - if result.len > 1: result.add(", ") + if first: first = false + else: result.add(", ") result.addQuoted(key) result.add(": ") result.addQuoted(val) diff --git a/lib/pure/collections/deques.nim b/lib/pure/collections/deques.nim index 5d67b361ea..0f797fad7d 100644 --- a/lib/pure/collections/deques.nim +++ b/lib/pure/collections/deques.nim @@ -454,8 +454,10 @@ proc `$`*[T](deq: Deque[T]): string = assert $a == "[10, 20, 30]" result = "[" + var first = true for x in deq: - if result.len > 1: result.add(", ") + if first: first = false + else: result.add(", ") result.addQuoted(x) result.add("]") diff --git a/lib/pure/collections/heapqueue.nim b/lib/pure/collections/heapqueue.nim index e83e5abbef..e2b6a6b52a 100644 --- a/lib/pure/collections/heapqueue.nim +++ b/lib/pure/collections/heapqueue.nim @@ -260,7 +260,9 @@ proc `$`*[T](heap: HeapQueue[T]): string = assert $heap == "[1, 2]" result = "[" + var first = true for x in heap.data: - if result.len > 1: result.add(", ") + if first: first = false + else: result.add(", ") result.addQuoted(x) result.add("]") diff --git a/lib/pure/collections/lists.nim b/lib/pure/collections/lists.nim index 6e7de204f1..7ca2242e5f 100644 --- a/lib/pure/collections/lists.nim +++ b/lib/pure/collections/lists.nim @@ -304,8 +304,10 @@ proc `$`*[T](L: SomeLinkedCollection[T]): string = assert $a == "[1, 2, 3, 4]" result = "[" + var first = true for x in nodes(L): - if result.len > 1: result.add(", ") + if first: first = false + else: result.add(", ") result.addQuoted(x.value) result.add("]") diff --git a/lib/pure/strtabs.nim b/lib/pure/strtabs.nim index 4b07aca5a3..663d4c832b 100644 --- a/lib/pure/strtabs.nim +++ b/lib/pure/strtabs.nim @@ -380,8 +380,10 @@ proc `$`*(t: StringTableRef): string {.rtlFunc, extern: "nstDollar".} = result = "{:}" else: result = "{" + var first = true for key, val in pairs(t): - if result.len > 1: result.add(", ") + if first: first = false + else: result.add(", ") result.add(key) result.add(": ") result.add(val) diff --git a/tests/stdlib/tdeques.nim b/tests/stdlib/tdeques.nim index 7d379a5975..e1ad50a84c 100644 --- a/tests/stdlib/tdeques.nim +++ b/tests/stdlib/tdeques.nim @@ -241,3 +241,12 @@ proc main() = static: main() main() + +# https://github.com/nim-lang/Nim/issues/18583 +# $ separator must be emitted even when the item's string repr is empty +type EmptyStr18583 = object +proc `$`(x: EmptyStr18583): string = "" + +block: + var d = [EmptyStr18583(), EmptyStr18583()].toDeque + doAssert $d == "[, ]", "got: " & $d diff --git a/tests/stdlib/theapqueue.nim b/tests/stdlib/theapqueue.nim index afb09c7e3f..92964c5641 100644 --- a/tests/stdlib/theapqueue.nim +++ b/tests/stdlib/theapqueue.nim @@ -104,3 +104,15 @@ template main() = static: main() main() + +# https://github.com/nim-lang/Nim/issues/18583 +type EmptyStr18583HeapQ = object +proc `$`(x: EmptyStr18583HeapQ): string = "" +proc `<`(a, b: EmptyStr18583HeapQ): bool = false + +block: + var h = initHeapQueue[EmptyStr18583HeapQ]() + push(h, EmptyStr18583HeapQ()) + push(h, EmptyStr18583HeapQ()) + let s = $h + doAssert s == "[, ]", "got: " & s diff --git a/tests/stdlib/tlists.nim b/tests/stdlib/tlists.nim index 9339a6df05..cffb00d586 100644 --- a/tests/stdlib/tlists.nim +++ b/tests/stdlib/tlists.nim @@ -287,3 +287,14 @@ template main = static: main() main() + +# https://github.com/nim-lang/Nim/issues/18583 +type EmptyStr18583List = object +proc `$`(x: EmptyStr18583List): string = "" + +block: + var L: SinglyLinkedList[EmptyStr18583List] + L.prepend(EmptyStr18583List()) + L.prepend(EmptyStr18583List()) + let s = $L + doAssert s == "[, ]", "got: " & s