Skip to content

Route IBufferWriterExtensions.Write through BuffersExtensions - #1209

Open
mgravell wants to merge 1 commit into
CommunityToolkit:mainfrom
mgravell:fix/ibufferwriter-write
Open

Route IBufferWriterExtensions.Write through BuffersExtensions#1209
mgravell wants to merge 1 commit into
CommunityToolkit:mainfrom
mgravell:fix/ibufferwriter-write

Conversation

@mgravell

@mgravell mgravell commented Aug 13, 2026

Copy link
Copy Markdown

Closes #1208

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>): type-pun, then delegate
  • Write<T>(IBufferWriter<byte>, T): delegate (type-pun via span)
  • 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.

Note on OverloadResolutionPriority.

Pre-empting an obvious question: we could in principle apply [OverloadResolutionPriority(-1)] instead of removing the this. It doesn't work here.

  • Priority is compared only within a declaring type: the spec groups candidates by declaring type, drops the lower-priority members inside each group, then recombines. The LDM confirmed this explicitly for extension methods ("we will always group"). BuffersExtensions and IBufferWriterExtensions are different types, so priority never enters the comparison - the ambiguity is still CS0121, and the byte-version still wins the hijack on specificity (concrete receiver beats generic receiver). Verified at C# 13.
  • It would also need a polyfilled attribute (PolySharp 1.15 doesn't offer it; this package opts into three generated types) and C# 13 in both the package build and the consumer. LangVersion here 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

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • Tested code with current supported SDKs
  • New component
    • Pull 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)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes
    • caveat: the changes to 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-ambiguity
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Other information

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IBufferWriterExtensions.Write : actively harmful

1 participant