From c6d395d04c2de467673caba60ed46e5ce57ede1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Sun, 9 Mar 2025 13:57:11 +0100 Subject: [PATCH 1/2] Specialize Memoize for IList and ICollection --- .../EnumerableExtensions/MemoizeTest.cs | 18 +++- Funcky/Buffers/CollectionBuffer.cs | 84 +++++++++++++++++++ Funcky/Buffers/ListBuffer.cs | 42 ++++++++++ .../EnumerableExtensions/Memoize.cs | 12 ++- 4 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 Funcky/Buffers/CollectionBuffer.cs create mode 100644 Funcky/Buffers/ListBuffer.cs diff --git a/Funcky.Test/Extensions/EnumerableExtensions/MemoizeTest.cs b/Funcky.Test/Extensions/EnumerableExtensions/MemoizeTest.cs index c4870b78..1659cb44 100644 --- a/Funcky.Test/Extensions/EnumerableExtensions/MemoizeTest.cs +++ b/Funcky.Test/Extensions/EnumerableExtensions/MemoizeTest.cs @@ -28,7 +28,7 @@ public void TheUnderlyingEnumerableIsOnlyEnumeratedOnce() [Fact] public void MemoizingAnEmptyListIsEmpty() { - var empty = Enumerable.Empty(); + var empty = Enumerable.Empty().PreventLinqOptimizations(); using var memoized = empty.Memoize(); Assert.Empty(memoized); @@ -88,4 +88,20 @@ public void MemoizingAMemoizedBufferTwiceReturnsTheOriginalObject() using var memoizedBuffer2 = memoizedBuffer.Memoize(); Assert.Same(memoizedBuffer, memoizedBuffer2); } + + [Fact] + public void MemoizingAListReturnsAnObjectImplementingIList() + { + var source = new List { 10, 20, 30 }; + using var memoized = source.Memoize(); + Assert.IsType>(memoized, exactMatch: false); + } + + [Fact] + public void MemoizingACollectionReturnsAnObjectImplementingICollection() + { + var source = new HashSet { 10, 20, 30 }; + using var memoized = source.Memoize(); + Assert.IsType>(memoized, exactMatch: false); + } } diff --git a/Funcky/Buffers/CollectionBuffer.cs b/Funcky/Buffers/CollectionBuffer.cs new file mode 100644 index 00000000..6c2dc127 --- /dev/null +++ b/Funcky/Buffers/CollectionBuffer.cs @@ -0,0 +1,84 @@ +using System.Collections; +using System.Diagnostics.CodeAnalysis; + +namespace Funcky.Buffers; + +internal static class CollectionBuffer +{ + public static CollectionBuffer Create(ICollection list) => new(list); +} + +[SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP025:Class with no virtual dispose method should be sealed", Justification = "Dispose doesn't do anything except flag object as disposed")] +internal class CollectionBuffer(ICollection source) : IBuffer, ICollection +{ + private bool _disposed; + + public int Count + { + get + { + ThrowIfDisposed(); + return source.Count; + } + } + + public bool IsReadOnly + { + get + { + ThrowIfDisposed(); + return source.IsReadOnly; + } + } + + public void Dispose() + { + _disposed = true; + } + + public IEnumerator GetEnumerator() + { + ThrowIfDisposed(); + return source.GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + + public void Add(T item) + { + ThrowIfDisposed(); + source.Add(item); + } + + public void Clear() + { + ThrowIfDisposed(); + source.Clear(); + } + + public bool Contains(T item) + { + ThrowIfDisposed(); + return source.Contains(item); + } + + public void CopyTo(T[] array, int arrayIndex) + { + ThrowIfDisposed(); + source.CopyTo(array, arrayIndex); + } + + public bool Remove(T item) + { + ThrowIfDisposed(); + return source.Remove(item); + } + + protected void ThrowIfDisposed() + { + if (_disposed) + { + throw new ObjectDisposedException(nameof(ListBuffer)); + } + } +} diff --git a/Funcky/Buffers/ListBuffer.cs b/Funcky/Buffers/ListBuffer.cs new file mode 100644 index 00000000..6add4c32 --- /dev/null +++ b/Funcky/Buffers/ListBuffer.cs @@ -0,0 +1,42 @@ +namespace Funcky.Buffers; + +internal static class ListBuffer +{ + public static ListBuffer Create(IList list) => new(list); +} + +internal sealed class ListBuffer(IList source) : CollectionBuffer(source), IList +{ + public T this[int index] + { + get + { + ThrowIfDisposed(); + return source[index]; + } + + set + { + ThrowIfDisposed(); + source[index] = value; + } + } + + public int IndexOf(T item) + { + ThrowIfDisposed(); + return source.IndexOf(item); + } + + public void Insert(int index, T item) + { + ThrowIfDisposed(); + source.Insert(index, item); + } + + public void RemoveAt(int index) + { + ThrowIfDisposed(); + source.RemoveAt(index); + } +} diff --git a/Funcky/Extensions/EnumerableExtensions/Memoize.cs b/Funcky/Extensions/EnumerableExtensions/Memoize.cs index 765a9a84..3cfa5390 100644 --- a/Funcky/Extensions/EnumerableExtensions/Memoize.cs +++ b/Funcky/Extensions/EnumerableExtensions/Memoize.cs @@ -1,6 +1,6 @@ -#pragma warning disable SA1010 // StyleCop support for collection expressions is missing using System.Collections; using System.Diagnostics.CodeAnalysis; +using Funcky.Buffers; namespace Funcky.Extensions; @@ -15,9 +15,13 @@ public static partial class EnumerableExtensions /// A lazy buffer of the underlying sequence. [Pure] public static IBuffer Memoize(this IEnumerable source) - => source is IBuffer buffer - ? Borrow(buffer) - : MemoizedBuffer.Create(source); + => source switch + { + IBuffer buffer => Borrow(buffer), + IList list => ListBuffer.Create(list), + ICollection list => CollectionBuffer.Create(list), + _ => MemoizedBuffer.Create(source), + }; [SuppressMessage("IDisposableAnalyzers", "IDISP015: Member should not return created and cached instance.", Justification = "False positive.")] private static IBuffer Borrow(IBuffer buffer) From 486b1835aeea30d1bd377bcd51f2b6a9a433fa8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Sun, 9 Mar 2025 13:57:38 +0100 Subject: [PATCH 2/2] Do not re-use borrowed buffer, always create a new one --- .../AsyncEnumerableExtensions/MemoizeTest.cs | 30 +++++++++++++++---- .../AsyncEnumerableExtensions/Memoize.cs | 2 +- .../EnumerableExtensions/MemoizeTest.cs | 28 +++++++++++++---- .../EnumerableExtensions/Memoize.cs | 4 +-- 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/Funcky.Async.Test/Extensions/AsyncEnumerableExtensions/MemoizeTest.cs b/Funcky.Async.Test/Extensions/AsyncEnumerableExtensions/MemoizeTest.cs index e92322a3..3628b502 100644 --- a/Funcky.Async.Test/Extensions/AsyncEnumerableExtensions/MemoizeTest.cs +++ b/Funcky.Async.Test/Extensions/AsyncEnumerableExtensions/MemoizeTest.cs @@ -80,12 +80,30 @@ public async Task DisposingAMemoizedBufferDoesNotDisposeOriginalBuffer() } [Fact] - public async Task MemoizingAMemoizedBufferTwiceReturnsTheOriginalObject() + public async Task DisposingAMemoizedBorrowedBufferDoesNotDisposeOriginalBorrowedBuffer() { - var source = AsyncEnumerateOnce.Create(Enumerable.Empty()); - await using var memoized = source.Memoize(); - await using var memoizedBuffer = memoized.Memoize(); - await using var memoizedBuffer2 = memoizedBuffer.Memoize(); - Assert.Same(memoizedBuffer, memoizedBuffer2); + var source = AsyncEnumerateOnce.Create([]); + await using var firstMemoization = source.Memoize(); + await using var borrowedBuffer = firstMemoization.Memoize(); + + await using (borrowedBuffer.Memoize()) + { + } + + await borrowedBuffer.ForEachAsync(NoOperation); + } + + /// This test disallows "re-borrowing" i.e. creating a fresh BorrowedBuffer over the original buffer. + [Fact] + public async Task UsagesOfSecondBorrowThrowAfterFirstBorrowIsDisposed() + { + var source = AsyncEnumerateOnce.Create([]); + await using var firstMemoization = source.Memoize(); + await using var firstBorrow = firstMemoization.Memoize(); + await using var secondBorrow = firstBorrow.Memoize(); +#pragma warning disable IDISP017 + await firstBorrow.DisposeAsync(); +#pragma warning restore IDISP017 + await Assert.ThrowsAsync(async () => await secondBorrow.ForEachAsync(NoOperation)); } } diff --git a/Funcky.Async/Extensions/AsyncEnumerableExtensions/Memoize.cs b/Funcky.Async/Extensions/AsyncEnumerableExtensions/Memoize.cs index 58f7b2a8..fa66e075 100644 --- a/Funcky.Async/Extensions/AsyncEnumerableExtensions/Memoize.cs +++ b/Funcky.Async/Extensions/AsyncEnumerableExtensions/Memoize.cs @@ -17,7 +17,7 @@ public static IAsyncBuffer Memoize(this IAsyncEnumerable Borrow(IAsyncBuffer buffer) - => buffer as BorrowedAsyncBuffer ?? new BorrowedAsyncBuffer(buffer); + => new BorrowedAsyncBuffer(buffer); private static class MemoizedAsyncBuffer { diff --git a/Funcky.Test/Extensions/EnumerableExtensions/MemoizeTest.cs b/Funcky.Test/Extensions/EnumerableExtensions/MemoizeTest.cs index 1659cb44..e70ccd34 100644 --- a/Funcky.Test/Extensions/EnumerableExtensions/MemoizeTest.cs +++ b/Funcky.Test/Extensions/EnumerableExtensions/MemoizeTest.cs @@ -80,13 +80,31 @@ public void DisposingAMemoizedBufferDoesNotDisposeOriginalBuffer() } [Fact] - public void MemoizingAMemoizedBufferTwiceReturnsTheOriginalObject() + public void DisposingAMemoizedBorrowedBufferDoesNotDisposeOriginalBorrowedBuffer() { var source = EnumerateOnce.Create([]); - using var memoized = source.Memoize(); - using var memoizedBuffer = memoized.Memoize(); - using var memoizedBuffer2 = memoizedBuffer.Memoize(); - Assert.Same(memoizedBuffer, memoizedBuffer2); + using var firstMemoization = source.Memoize(); + using var borrowedBuffer = firstMemoization.Memoize(); + + using (borrowedBuffer.Memoize()) + { + } + + borrowedBuffer.ForEach(NoOperation); + } + + /// This test disallows "re-borrowing" i.e. creating a fresh BorrowedBuffer over the original buffer. + [Fact] + public void UsagesOfSecondBorrowThrowAfterFirstBorrowIsDisposed() + { + var source = EnumerateOnce.Create([]); + using var firstMemoization = source.Memoize(); + using var firstBorrow = firstMemoization.Memoize(); + using var secondBorrow = firstBorrow.Memoize(); +#pragma warning disable IDISP017 + firstBorrow.Dispose(); +#pragma warning restore IDISP017 + Assert.Throws(() => secondBorrow.ForEach(NoOperation)); } [Fact] diff --git a/Funcky/Extensions/EnumerableExtensions/Memoize.cs b/Funcky/Extensions/EnumerableExtensions/Memoize.cs index 3cfa5390..b4aaec05 100644 --- a/Funcky/Extensions/EnumerableExtensions/Memoize.cs +++ b/Funcky/Extensions/EnumerableExtensions/Memoize.cs @@ -1,5 +1,4 @@ using System.Collections; -using System.Diagnostics.CodeAnalysis; using Funcky.Buffers; namespace Funcky.Extensions; @@ -23,9 +22,8 @@ public static IBuffer Memoize(this IEnumerable source _ => MemoizedBuffer.Create(source), }; - [SuppressMessage("IDisposableAnalyzers", "IDISP015: Member should not return created and cached instance.", Justification = "False positive.")] private static IBuffer Borrow(IBuffer buffer) - => buffer as BorrowedBuffer ?? new BorrowedBuffer(buffer); + => new BorrowedBuffer(buffer); private static class MemoizedBuffer {