Skip to content

Commit 757c6fd

Browse files
lookbusy1344claude
andcommitted
Fix empty PooledString handling and add comprehensive insert tests
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 82158d2 commit 757c6fd

File tree

8 files changed

+64
-38
lines changed

8 files changed

+64
-38
lines changed

Demo/Demo.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public static void Main()
6060

6161
Console.WriteLine($"Buffer dump: \"{pool.DumpBufferAsString()}\"");
6262

63-
var last = PooledString.Empty;
63+
var last = pool.CreateEmptyString();
6464
for (var i = 0; i < 100; ++i) {
6565
var randomStr = RandomString(Random.Shared.Next(5, 150));
6666
var pooledStr = pool.Allocate(randomStr);

PooledString.cs

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,6 @@ public readonly record struct PooledString(UnmanagedStringPool Pool, uint Alloca
2626
// NOTE this struct is technically immutable, but some methods mutate the underlying pool like SetAtPosition() and Free()
2727
// It also implements IDisposable to call Free() automatically
2828

29-
/// <summary>
30-
/// Represents an empty pooled string
31-
/// </summary>
32-
public static readonly PooledString Empty = new(null!, UnmanagedStringPool.EmptyStringAllocationId);
33-
3429
#region Public API
3530

3631
/// <summary>
@@ -64,13 +59,8 @@ public readonly PooledString Insert(int pos, ReadOnlySpan<char> value)
6459
return this;
6560
}
6661

67-
if (AllocationId == UnmanagedStringPool.EmptyStringAllocationId) {
68-
if (pos != 0) {
69-
throw new ArgumentOutOfRangeException(nameof(pos), "Cannot insert into an empty string at position other than 0");
70-
}
71-
72-
throw new InvalidOperationException(
73-
"Cannot insert into PooledString.Empty without a valid pool. Use pool.Allocate() to create a new string.");
62+
if (AllocationId == UnmanagedStringPool.EmptyStringAllocationId && pos != 0) {
63+
throw new ArgumentOutOfRangeException(nameof(pos), "Cannot insert into an empty string at position other than 0");
7464
}
7565

7666
CheckDisposed();
@@ -356,8 +346,7 @@ public override readonly int GetHashCode()
356346
/// </summary>
357347
private readonly void CheckDisposed()
358348
{
359-
// Empty string is always valid, no pool needed
360-
if (AllocationId != UnmanagedStringPool.EmptyStringAllocationId && Pool?.IsDisposed != false) {
349+
if (Pool.IsDisposed) {
361350
throw new ObjectDisposedException(nameof(PooledString));
362351
}
363352
}

Tests/ConcurrentAccessTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ public async Task ConcurrentEmptyStringAccess_AlwaysThreadSafeAsync()
430430
tasks[t] = Task.Run(() => {
431431
try {
432432
for (var i = 0; i < 200; i++) {
433-
var empty = PooledString.Empty;
433+
var empty = pool.CreateEmptyString();
434434

435435
// All these operations should be thread-safe on empty strings
436436
var span = empty.AsSpan();

Tests/DisposalAndLifecycleTests.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,8 @@ public void PooledString_UseAfterFree_ThrowsException()
236236
[Fact]
237237
public void EmptyString_Dispose_DoesNotThrow()
238238
{
239-
var empty = PooledString.Empty;
239+
using var pool = new UnmanagedStringPool(1024);
240+
var empty = pool.CreateEmptyString();
240241

241242
empty.Free();
242243
empty.Dispose();
@@ -247,7 +248,8 @@ public void EmptyString_Dispose_DoesNotThrow()
247248
[Fact]
248249
public void EmptyString_MultipleDispose_Safe()
249250
{
250-
var empty = PooledString.Empty;
251+
using var pool = new UnmanagedStringPool(1024);
252+
var empty = pool.CreateEmptyString();
251253

252254
for (var i = 0; i < 10; i++) {
253255
empty.Free();
@@ -263,8 +265,9 @@ public void EmptyString_MultipleDispose_Safe()
263265
[Fact]
264266
public void EmptyString_AlwaysValid_EvenAfterMultipleDisposals()
265267
{
266-
var empty1 = PooledString.Empty;
267-
var empty2 = PooledString.Empty;
268+
using var pool = new UnmanagedStringPool(1024);
269+
var empty1 = pool.CreateEmptyString();
270+
var empty2 = pool.CreateEmptyString();
268271

269272
empty1.Dispose();
270273
empty2.Free();

Tests/IntegerOverflowTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ public void BufferMemoryCopy_LengthCalculations_HandleBoundaries()
337337
foreach (var testStr in testStrings) {
338338
PooledString result;
339339
if (string.IsNullOrEmpty(testStr)) {
340-
// Empty string case: allocate "PREFIX" directly since pool.Allocate("") returns PooledString.Empty
340+
// Empty string case: allocate "PREFIX" directly since we can't insert into an empty string from a different pool
341341
result = pool.Allocate("PREFIX");
342342
} else {
343343
var str = pool.Allocate(testStr);

Tests/PooledStringTests.cs

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,8 @@ public void Insert_EmptyValue_ReturnsOriginal()
6060
[Fact]
6161
public void Insert_IntoEmptyString_WorksCorrectly()
6262
{
63-
var empty = pool.Allocate("");
64-
// Since pool.Allocate("") returns PooledString.Empty, we need to allocate through the pool directly
65-
var result = pool.Allocate("Hello");
63+
var empty = pool.CreateEmptyString();
64+
var result = empty.Insert(0, "Hello");
6665

6766
Assert.Equal("Hello", result.ToString());
6867
}
@@ -80,11 +79,38 @@ public void Insert_InvalidPosition_ThrowsArgumentOutOfRangeException(int positio
8079
[Fact]
8180
public void Insert_IntoEmptyStringInvalidPosition_ThrowsArgumentOutOfRangeException()
8281
{
83-
var empty = PooledString.Empty;
82+
var empty = pool.CreateEmptyString();
8483

8584
Assert.Throws<ArgumentOutOfRangeException>(() => empty.Insert(1, "test"));
8685
}
8786

87+
[Fact]
88+
public void Insert_IntoAllocatedEmptyString_WorksCorrectly()
89+
{
90+
var empty = pool.Allocate("");
91+
var result = empty.Insert(0, "World");
92+
93+
Assert.Equal("World", result.ToString());
94+
}
95+
96+
[Fact]
97+
public void Insert_AppendToString_WorksCorrectly()
98+
{
99+
var str = pool.Allocate("Hello");
100+
var result = str.Insert(str.Length, " World");
101+
102+
Assert.Equal("Hello World", result.ToString());
103+
}
104+
105+
[Fact]
106+
public void Insert_AppendToEmptyString_WorksCorrectly()
107+
{
108+
var empty = pool.CreateEmptyString();
109+
var result = empty.Insert(empty.Length, "Appended");
110+
111+
Assert.Equal("Appended", result.ToString());
112+
}
113+
88114
#endregion
89115

90116
#region Replace Tests
@@ -172,6 +198,7 @@ public void Replace_EntireString_WorksCorrectly()
172198

173199
#endregion
174200

201+
175202
#region Search Operations Tests
176203

177204
[Fact]
@@ -203,7 +230,7 @@ public void IndexOf_EmptyString_ReturnsZero()
203230
[Fact]
204231
public void IndexOf_InEmptyString_ReturnsCorrectly()
205232
{
206-
var empty = PooledString.Empty;
233+
var empty = pool.CreateEmptyString();
207234

208235
Assert.Equal(0, empty.IndexOf(""));
209236
Assert.Equal(-1, empty.IndexOf("test"));
@@ -303,7 +330,7 @@ public void EndsWith_Incorrect_ReturnsFalse()
303330
[Fact]
304331
public void StringOperations_WithEmptyString_WorkCorrectly()
305332
{
306-
var empty = PooledString.Empty;
333+
var empty = pool.CreateEmptyString();
307334

308335
Assert.True(empty.Contains(""));
309336
Assert.True(empty.StartsWith(""));
@@ -382,7 +409,7 @@ public void AsSpan_ValidString_ReturnsCorrectSpan()
382409
[Fact]
383410
public void AsSpan_EmptyString_ReturnsEmptySpan()
384411
{
385-
var empty = PooledString.Empty;
412+
var empty = pool.CreateEmptyString();
386413
var span = empty.AsSpan();
387414

388415
Assert.True(span.IsEmpty);
@@ -425,8 +452,9 @@ public void Equals_DifferentContent_ReturnsFalse()
425452
[Fact]
426453
public void Equals_EmptyStrings_ReturnsTrue()
427454
{
428-
var empty1 = PooledString.Empty;
429-
var empty2 = PooledString.Empty;
455+
using var pool = new UnmanagedStringPool(1024);
456+
var empty1 = pool.CreateEmptyString();
457+
var empty2 = pool.CreateEmptyString();
430458

431459
Assert.True(empty1.Equals(empty2));
432460
Assert.True(empty1 == empty2);
@@ -435,7 +463,7 @@ public void Equals_EmptyStrings_ReturnsTrue()
435463
[Fact]
436464
public void Equals_EmptyWithAllocated_WorksCorrectly()
437465
{
438-
var empty = PooledString.Empty;
466+
var empty = pool.CreateEmptyString();
439467
var emptyAllocated = pool.Allocate("");
440468
var nonEmpty = pool.Allocate("Hello");
441469

@@ -501,7 +529,7 @@ public void GetHashCode_RandomStrings_MostHaveDifferentHashes()
501529
[Fact]
502530
public void GetHashCode_EmptyString_ReturnsZero()
503531
{
504-
var empty = PooledString.Empty;
532+
var empty = pool.CreateEmptyString();
505533

506534
Assert.Equal(0, empty.GetHashCode());
507535
}
@@ -639,7 +667,7 @@ public void Length_CorrectlyReturnsStringLength()
639667
[Fact]
640668
public void Length_EmptyString_ReturnsZero()
641669
{
642-
var empty = PooledString.Empty;
670+
var empty = pool.CreateEmptyString();
643671
var emptyAllocated = pool.Allocate("");
644672

645673
Assert.Equal(0, empty.Length);
@@ -649,7 +677,7 @@ public void Length_EmptyString_ReturnsZero()
649677
[Fact]
650678
public void IsEmpty_EmptyString_ReturnsTrue()
651679
{
652-
var empty = PooledString.Empty;
680+
var empty = pool.CreateEmptyString();
653681
var emptyAllocated = pool.Allocate("");
654682

655683
Assert.True(empty.IsEmpty);
@@ -679,7 +707,7 @@ public void ToString_ValidString_ReturnsCorrectString()
679707
[Fact]
680708
public void ToString_EmptyString_ReturnsEmptyString()
681709
{
682-
var empty = PooledString.Empty;
710+
var empty = pool.CreateEmptyString();
683711

684712
Assert.Equal("", empty.ToString());
685713
}

Tests/UnmanagedStringPoolTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ public void AsSpan_AfterPoolDisposal_ThrowsObjectDisposedException()
221221
[Fact]
222222
public void FreeString_EmptyString_NoEffect()
223223
{
224-
var emptyStr = PooledString.Empty;
224+
var emptyStr = pool.CreateEmptyString();
225225

226226
// Should not throw or cause issues
227227
emptyStr.Free();

UnmanagedStringPool.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public PooledString Allocate(ReadOnlySpan<char> value)
144144
ObjectDisposedException.ThrowIf(IsDisposed, typeof(UnmanagedStringPool));
145145

146146
if (value.IsEmpty) {
147-
return PooledString.Empty;
147+
return CreateEmptyString();
148148
}
149149

150150
// allocate a buffer for this string
@@ -274,7 +274,7 @@ internal PooledString Allocate(int lengthChars)
274274
ObjectDisposedException.ThrowIf(IsDisposed, typeof(UnmanagedStringPool));
275275

276276
if (lengthChars <= 0) {
277-
return PooledString.Empty;
277+
return CreateEmptyString();
278278
}
279279

280280
// Check for overflow when converting to bytes and aligning
@@ -367,6 +367,12 @@ internal void FreeString(uint id)
367367
}
368368
}
369369

370+
/// <summary>
371+
/// Create an empty PooledString in this pool. Requires a pool so we know where to add if insertion occurs
372+
/// </summary>
373+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
374+
internal PooledString CreateEmptyString() => new(this, EmptyStringAllocationId);
375+
370376
#endregion
371377

372378
#region Private methods

0 commit comments

Comments
 (0)