Route IBufferWriterExtensions.Write through BuffersExtensions - #1209
Open
mgravell wants to merge 1 commit into
Open
Route IBufferWriterExtensions.Write through BuffersExtensions#1209mgravell wants to merge 1 commit into
mgravell wants to merge 1 commit into
Conversation
sizeHint is a hint, not a demand: a writer may hand back less than was asked for, so requiring an entire payload in one contiguous span fails against writers with bounded segments, where BuffersExtensions loops. - Write<T>(IBufferWriter<byte>, ReadOnlySpan<T>): blit, then delegate - Write<T>(IBufferWriter<byte>, T): delegate - Write<T>(IBufferWriter<T>, ReadOnlySpan<T>) (netstandard2.0 only): delegate, mark [Obsolete], and drop the `this`; it has the same signature as the BCL method, so as an extension method it was only ever an ambiguity (CS0121). Retained for binary compatibility. Write<T>(IBufferWriter<T>, T) is left alone: GetSpan(1) plus a length check is defensible, as a writer that cannot supply a single element is broken under any reading of the contract. Tests use a SegmentedWriter<T> that never hands out more than eight elements per GetSpan call, whatever the hint asks for. The using System.Buffers; in the test file no longer needs a #if to dodge the ambiguity, which is itself part of the regression test. See CommunityToolkit#1208
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1208
sizeHintis a hint, not a demand: a writer may hand back less than was asked for, so requiring an entire payload in one contiguous span fails against writers with bounded segments, whereBuffersExtensionsloops.Write<T>(IBufferWriter<byte>, ReadOnlySpan<T>): type-pun, then delegateWrite<T>(IBufferWriter<byte>, T): delegate (type-pun via span)Write<T>(IBufferWriter<T>, ReadOnlySpan<T>)(netstandard2.0 only): delegate, mark[Obsolete], and drop thethis; it has the same signature as the BCL method, so as an extension method it was only ever an ambiguity (CS0121). Retained for binary compatibility.Write<T>(IBufferWriter<T>, T)is left alone:GetSpan(1)plus a length check is defensible, as a writer that cannot supply a single element is broken under any reading of the contract.Tests use a
SegmentedWriter<T>that never hands out more than eight elements perGetSpancall, whatever the hint asks for. Theusing System.Buffers;in the test file no longer needs a#ifto dodge the ambiguity, which is itself part of the regression test.Note on
OverloadResolutionPriority.Pre-empting an obvious question: we could in principle apply
[OverloadResolutionPriority(-1)]instead of removing thethis. It doesn't work here.BuffersExtensionsandIBufferWriterExtensionsare different types, so priority never enters the comparison - the ambiguity is still CS0121, and thebyte-version still wins the hijack on specificity (concrete receiver beats generic receiver). Verified at C# 13.LangVersionhere is currently 12. A consumer on an older compiler ignores the attribute silently rather than failing, so the fix would do nothing for exactly the down-level consumers this package exists to serve.PR Checklist
New componentPull Request has been submitted to the documentation repository instructions.Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)Header has been added to all new source files (run build/UpdateHeaders.bat)Write<T>(IBufferWriter<T>, ReadOnlySpan<T>)are build-time breaking, intentionally and necessarily, but not runtime breaking; a one-time "migrate to System.Buffers" is less harmful than a forever overload-ambiguityEvery new API (including internal ones) has full XML docsOther information