Consolidate downlevel polyfills under Common/src/Polyfills

[Copilot]

Follow-up to #126287 — consolidates library-local BitOperations, Stream, StringBuilder, and Stack polyfills under src/libraries/Common/src/Polyfills/.

Changes

Common/src/Polyfills/BitOperationsPolyfills.cs

Expanded the existing polyfill (which only had RotateLeft(uint)) with the full set of methods needed by the affected libraries:

• Log2(uint) (deBruijn-based)
• PopCount(uint) and PopCount(ulong) (Hamming weight)
• RotateLeft(ulong), RotateRight(uint), RotateRight(ulong)
• TrailingZeroCount(uint) (deBruijn-based)

Common/src/Polyfills/StreamPolyfills.cs

Merged ReadExactly and CopyToAsync from the now-deleted Common/src/System/IO/StreamExtensions.netstandard.cs into the existing Stream polyfill (no System.Memory/System.Buffers dependencies).

Common/src/Polyfills/StreamMemoryPolyfills.cs(new)

New file containing the Memory-dependent stream extension methods, split out from StreamPolyfills.cs to avoid pulling in extra package references for consumers that don't need them:

• ReadAsync(Memory<byte>, CancellationToken)
• Write(ReadOnlyMemory<byte>)
• WriteAsync(ReadOnlyMemory<byte>, CancellationToken)

Common/src/Polyfills/StringBuilderPolyfills.cs(new)

Moved from Common/src/System/Text/StringBuilderExtensions.cs and reworked to follow polyfill directory conventions (file-scoped namespace, extension(StringBuilder) block syntax):

• Append(ReadOnlySpan<char>)

Common/src/Polyfills/StackPolyfills.cs(new)

Moved from System.Text.Json-local StackExtensions.netstandard.cs and reworked to follow polyfill directory conventions (file-scoped namespace, traditional extension methods matching DictionaryPolyfills.cs style for generic types):

• TryPeek<T>(out T)
• TryPop<T>(out T)

System.IO.Hashing

• Removed local System/IO/Hashing/BitOperations.cs (had RotateLeft(uint/ulong))
• Updated .csproj to reference BitOperationsPolyfills.cs and StreamPolyfills.cs

System.Text.Encodings.Web

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had Log2)
• Updated .csproj to reference BitOperationsPolyfills.cs

Microsoft.Bcl.Memory

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had TrailingZeroCount, RotateLeft(uint), RotateRight(uint))
• Updated .csproj to reference BitOperationsPolyfills.cs (which is a strict superset)

System.Reflection.Metadata

• Deleted BitArithmetic.CountBits(uint/ulong) wrapper methods; all call sites (BitArithmetic.Align, MetadataSizes, PEHeaderBuilder) now call BitOperations.PopCount directly
• Updated TagToTokenTests.cs to call BitOperations.PopCount directly (fixes .NETFramework build break)
• Added BitOperationsPolyfills.cs include in .csproj for non-.NETCoreApp targets

System.Net.ServerSentEvents, System.Text.Json, Microsoft.Extensions.Logging.Console, System.Net.Http.WinHttpHandler.Functional.Tests

• Reference StreamMemoryPolyfills.cs (use Memory-based stream methods)

System.IO.Pipelines

• References both StreamPolyfills.cs (uses CopyToAsync) and StreamMemoryPolyfills.cs (uses ReadAsync(Memory<byte>))

System.Speech, System.ServiceModel.Syndication

• Reference StreamPolyfills.cs only (use ReadExactly); no extra System.Memory/System.Buffers/System.Threading.Tasks.Extensions package references needed

System.Text.Json

• Updated .csproj to reference StringBuilderPolyfills.cs and StackPolyfills.cs from Common/src/Polyfills/
• Removed local StackExtensions.netstandard.cs

Note

This PR description was generated with the assistance of GitHub Copilot.

[github-actions]

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #126391

Holistic Assessment

Motivation: This PR continues the consolidation work from #126287 by merging three separate library-local BitOperations polyfill implementations (from System.IO.Hashing, System.Text.Encodings.Web, and System.Reflection.Metadata) into the shared Common/src/Polyfills/BitOperationsPolyfills.cs. The motivation is clear and well-justified — eliminating duplicate code and reducing #if NET/#else blocks.

Approach: The approach is consistent with the prior PR's pattern: centralize polyfills in a shared directory, update .csproj files to link the shared file for non-.NETCoreApp targets, and remove library-local copies. The consolidation is straightforward.

Summary: ❌ Needs Changes. The PR has a clear compilation error: a double opening brace in the Log2 method body that leaves the method unclosed. This must be fixed before merge. All other changes are clean.

---

Detailed Findings

❌ Compilation Error — Double { in Log2 method (BitOperationsPolyfills.cs:32-33)

Merge-blocking. The Log2 method has two consecutive opening braces but only one closing brace, creating an unbalanced brace structure that will not compile:

publicstaticintLog2(uintvalue){// ← line 32: method body opens{// ← line 33: extra block opens (from the old Log2SoftwareFallback body)// ... implementation ...returnUnsafe.AddByteOffset(...);}// ← line 50: closes only the inner block// method body is never closed!

Brace analysis of the entire file shows 8 opening braces vs 7 closing braces (final nesting depth 1, expected 0). All subsequent methods (PopCount, RotateLeft, etc.) end up syntactically nested inside the unclosed Log2 body, which is invalid C# — you cannot declare public static members inside a method.

Root cause: Commit 7ee20d2d ("Apply suggestion from @jkotas") merged the Log2 wrapper and Log2SoftwareFallback into a single method, but the replacement added a new opening brace for the merged method without removing the existing opening brace from the old Log2SoftwareFallback body.

Fix: Remove the extra { on line 33:

publicstaticintLog2(uintvalue){// No AggressiveInlining due to large method size// Has conventional contract 0->0 (Log(0) is undefined)

✅ Correctness — value | 1 removal in Log2 is safe

The original System.Text.Encodings.Web implementation called Log2SoftwareFallback(value | 1) to guarantee the 0 → 0 contract. The merged version drops the | 1. This is safe: when value = 0, the OR-shift cascade leaves it at 0, the DeBruijn multiplication yields index 0, and Log2DeBruijn[0] = 0, so Log2(0) correctly returns 0. All other inputs are unaffected since | 1 only set the lowest bit which doesn't change floor(log2(x)) for x > 0.

✅ Correctness — Algorithm implementations match BCL software fallbacks

The PopCount(uint), PopCount(ulong), TrailingZeroCount(uint), RotateLeft, and RotateRight implementations are faithful reproductions of the well-known software fallbacks (Hamming weight, DeBruijn sequences, bit rotations). These match the deleted library-local versions exactly.

✅ Consolidation — csproj changes are correct

All three .csproj files correctly:

• Replace local file references with $(CommonPath)Polyfills\BitOperationsPolyfills.cs
• Scope the reference to '$(TargetFrameworkIdentifier)' != '.NETCoreApp'
• Use appropriate Link attributes for solution explorer organization

✅ Cleanup — BitArithmetic.cs#if NET removal is clean

Removing the #if NET/#else blocks in System.Reflection.Metadata's BitArithmetic.cs and unconditionally calling BitOperations.PopCount is correct — the polyfill provides the same implementation that was previously inlined under #else.

💡 Observation — Extra methods not currently consumed

The consolidated polyfill includes RotateRight(uint/ulong) and TrailingZeroCount(uint) which are not currently used by any of the three consuming libraries. This is fine for a shared polyfill (other libraries may need them in the future), but worth noting for awareness.

Generated by Code Review for issue #126391 · ◷

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR consolidates downlevel (TargetFrameworkIdentifier != .NETCoreApp) polyfills for System.Numerics.BitOperations and Memory-based System.IO.Stream APIs into src/libraries/Common/src/Polyfills/, and updates multiple library projects to consume the shared implementations instead of library-local copies.

Changes:

• Expanded Common’s BitOperations polyfill and switched affected libraries to reference it from Common/src/Polyfills.
• Split stream polyfills into StreamPolyfills (no System.Memory dependencies) and StreamMemoryPolyfills (Memory-based APIs) and updated consuming projects accordingly.
• Removed library-local polyfill sources (e.g., in System.IO.Hashing and System.Text.Encodings.Web) and updated call sites (e.g., System.Reflection.Metadata) to use BitOperations.PopCount.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Text.Json/src/System.Text.Json.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj	Replaces local BitOperations polyfill with shared BitOperationsPolyfills.
src/libraries/System.Text.Encodings.Web/src/Polyfills/System.Numerics.BitOperations.netstandard20.cs	Deletes library-local Log2 polyfill implementation.
src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs	Replaces BitArithmetic.CountBits usage with BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs	Switches external table count calculation to BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs	Removes CountBits helpers and uses BitOperations.PopCount for alignment asserts.
src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj	Adds shared BitOperationsPolyfills.cs for non-.NETCoreApp targets.
src/libraries/System.Net.ServerSentEvents/src/System.Net.ServerSentEvents.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj	Adds StreamPolyfills + StreamMemoryPolyfills for downlevel targets.
src/libraries/System.IO.Pipelines/src/System.IO.Pipelines.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/System.IO.Hashing/src/System/IO/Hashing/BitOperations.cs	Deletes library-local RotateLeft polyfill.
src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj	References shared BitOperationsPolyfills + StreamPolyfills for downlevel targets.
src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs	Adds Memory-based stream polyfills (ReadAsync, Write, WriteAsync) for downlevel TFMs.
src/libraries/Common/src/Polyfills/BitOperationsPolyfills.cs	Expands shared BitOperations polyfill surface (Log2/PopCount/rotates/TZC).

Comments suppressed due to low confidence (2)

src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:19

• In ReadAsync(Memory) the non-array-backed path rents a buffer from ArrayPool and then calls stream.ReadAsync(...) inline. If stream.ReadAsync throws synchronously (e.g., ObjectDisposedException / argument validation) before returning a Task, the rented buffer won't be returned and will leak. Consider capturing the Task inside a try/catch and returning the rented buffer in the catch before rethrowing, then passing the Task to FinishReadAsync for the awaited path.
  src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:76
• In WriteAsync(ReadOnlyMemory) the non-array-backed path rents a buffer and then calls stream.WriteAsync(...) without guarding against synchronous exceptions. If stream.WriteAsync throws before returning a Task, the rented buffer will leak because FinishWriteAsync won't run. Capture the Task in a try/catch (or use a try/finally around the call) and ensure ArrayPool.Return happens on the synchronous-throw path as well.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Fixed in commit d5a297c. Updated TagToTokenTests.cs to call BitOperations.PopCount(...) directly instead of the deleted BitArithmetic.CountBits(...), and added using System.Numerics;.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Already fixed in commit d5a297c — TagToTokenTests.cs now calls BitOperations.PopCount(...) directly with using System.Numerics;.

[Copilot]

Follow-up to #126287 — consolidates library-local BitOperations, Stream, StringBuilder, and Stack polyfills under src/libraries/Common/src/Polyfills/.

Changes

Common/src/Polyfills/BitOperationsPolyfills.cs

Expanded the existing polyfill (which only had RotateLeft(uint)) with the full set of methods needed by the affected libraries:

• Log2(uint) (deBruijn-based)
• PopCount(uint) and PopCount(ulong) (Hamming weight)
• RotateLeft(ulong), RotateRight(uint), RotateRight(ulong)
• TrailingZeroCount(uint) (deBruijn-based)

Common/src/Polyfills/StreamPolyfills.cs

Merged ReadExactly and CopyToAsync from the now-deleted Common/src/System/IO/StreamExtensions.netstandard.cs into the existing Stream polyfill (no System.Memory/System.Buffers dependencies).

Common/src/Polyfills/StreamMemoryPolyfills.cs(new)

New file containing the Memory-dependent stream extension methods, split out from StreamPolyfills.cs to avoid pulling in extra package references for consumers that don't need them:

• ReadAsync(Memory<byte>, CancellationToken)
• Write(ReadOnlyMemory<byte>)
• WriteAsync(ReadOnlyMemory<byte>, CancellationToken)

Common/src/Polyfills/StringBuilderPolyfills.cs(new)

Moved from Common/src/System/Text/StringBuilderExtensions.cs and reworked to follow polyfill directory conventions (file-scoped namespace, extension(StringBuilder) block syntax):

• Append(ReadOnlySpan<char>)

Common/src/Polyfills/StackPolyfills.cs(new)

Moved from System.Text.Json-local StackExtensions.netstandard.cs and reworked to follow polyfill directory conventions (file-scoped namespace, traditional extension methods matching DictionaryPolyfills.cs style for generic types):

• TryPeek<T>(out T)
• TryPop<T>(out T)

System.IO.Hashing

• Removed local System/IO/Hashing/BitOperations.cs (had RotateLeft(uint/ulong))
• Updated .csproj to reference BitOperationsPolyfills.cs and StreamPolyfills.cs

System.Text.Encodings.Web

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had Log2)
• Updated .csproj to reference BitOperationsPolyfills.cs

Microsoft.Bcl.Memory

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had TrailingZeroCount, RotateLeft(uint), RotateRight(uint))
• Updated .csproj to reference BitOperationsPolyfills.cs (which is a strict superset)

System.Reflection.Metadata

• Deleted BitArithmetic.CountBits(uint/ulong) wrapper methods; all call sites (BitArithmetic.Align, MetadataSizes, PEHeaderBuilder) now call BitOperations.PopCount directly
• Updated TagToTokenTests.cs to call BitOperations.PopCount directly (fixes .NETFramework build break)
• Added BitOperationsPolyfills.cs include in .csproj for non-.NETCoreApp targets

System.Net.ServerSentEvents, System.Text.Json, Microsoft.Extensions.Logging.Console, System.Net.Http.WinHttpHandler.Functional.Tests

• Reference StreamMemoryPolyfills.cs (use Memory-based stream methods)

System.IO.Pipelines

• References both StreamPolyfills.cs (uses CopyToAsync) and StreamMemoryPolyfills.cs (uses ReadAsync(Memory<byte>))

System.Speech, System.ServiceModel.Syndication

• Reference StreamPolyfills.cs only (use ReadExactly); no extra System.Memory/System.Buffers/System.Threading.Tasks.Extensions package references needed

System.Text.Json

• Updated .csproj to reference StringBuilderPolyfills.cs and StackPolyfills.cs from Common/src/Polyfills/
• Removed local StackExtensions.netstandard.cs

Note

This PR description was generated with the assistance of GitHub Copilot.

[github-actions]

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #126391

Holistic Assessment

Motivation: This PR continues the consolidation work from #126287 by merging three separate library-local BitOperations polyfill implementations (from System.IO.Hashing, System.Text.Encodings.Web, and System.Reflection.Metadata) into the shared Common/src/Polyfills/BitOperationsPolyfills.cs. The motivation is clear and well-justified — eliminating duplicate code and reducing #if NET/#else blocks.

Approach: The approach is consistent with the prior PR's pattern: centralize polyfills in a shared directory, update .csproj files to link the shared file for non-.NETCoreApp targets, and remove library-local copies. The consolidation is straightforward.

Summary: ❌ Needs Changes. The PR has a clear compilation error: a double opening brace in the Log2 method body that leaves the method unclosed. This must be fixed before merge. All other changes are clean.

---

Detailed Findings

❌ Compilation Error — Double { in Log2 method (BitOperationsPolyfills.cs:32-33)

Merge-blocking. The Log2 method has two consecutive opening braces but only one closing brace, creating an unbalanced brace structure that will not compile:

publicstaticintLog2(uintvalue){// ← line 32: method body opens{// ← line 33: extra block opens (from the old Log2SoftwareFallback body)// ... implementation ...returnUnsafe.AddByteOffset(...);}// ← line 50: closes only the inner block// method body is never closed!

Brace analysis of the entire file shows 8 opening braces vs 7 closing braces (final nesting depth 1, expected 0). All subsequent methods (PopCount, RotateLeft, etc.) end up syntactically nested inside the unclosed Log2 body, which is invalid C# — you cannot declare public static members inside a method.

Root cause: Commit 7ee20d2d ("Apply suggestion from @jkotas") merged the Log2 wrapper and Log2SoftwareFallback into a single method, but the replacement added a new opening brace for the merged method without removing the existing opening brace from the old Log2SoftwareFallback body.

Fix: Remove the extra { on line 33:

publicstaticintLog2(uintvalue){// No AggressiveInlining due to large method size// Has conventional contract 0->0 (Log(0) is undefined)

✅ Correctness — value | 1 removal in Log2 is safe

The original System.Text.Encodings.Web implementation called Log2SoftwareFallback(value | 1) to guarantee the 0 → 0 contract. The merged version drops the | 1. This is safe: when value = 0, the OR-shift cascade leaves it at 0, the DeBruijn multiplication yields index 0, and Log2DeBruijn[0] = 0, so Log2(0) correctly returns 0. All other inputs are unaffected since | 1 only set the lowest bit which doesn't change floor(log2(x)) for x > 0.

✅ Correctness — Algorithm implementations match BCL software fallbacks

The PopCount(uint), PopCount(ulong), TrailingZeroCount(uint), RotateLeft, and RotateRight implementations are faithful reproductions of the well-known software fallbacks (Hamming weight, DeBruijn sequences, bit rotations). These match the deleted library-local versions exactly.

✅ Consolidation — csproj changes are correct

All three .csproj files correctly:

• Replace local file references with $(CommonPath)Polyfills\BitOperationsPolyfills.cs
• Scope the reference to '$(TargetFrameworkIdentifier)' != '.NETCoreApp'
• Use appropriate Link attributes for solution explorer organization

✅ Cleanup — BitArithmetic.cs#if NET removal is clean

Removing the #if NET/#else blocks in System.Reflection.Metadata's BitArithmetic.cs and unconditionally calling BitOperations.PopCount is correct — the polyfill provides the same implementation that was previously inlined under #else.

💡 Observation — Extra methods not currently consumed

The consolidated polyfill includes RotateRight(uint/ulong) and TrailingZeroCount(uint) which are not currently used by any of the three consuming libraries. This is fine for a shared polyfill (other libraries may need them in the future), but worth noting for awareness.

Generated by Code Review for issue #126391 · ◷

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR consolidates downlevel (TargetFrameworkIdentifier != .NETCoreApp) polyfills for System.Numerics.BitOperations and Memory-based System.IO.Stream APIs into src/libraries/Common/src/Polyfills/, and updates multiple library projects to consume the shared implementations instead of library-local copies.

Changes:

• Expanded Common’s BitOperations polyfill and switched affected libraries to reference it from Common/src/Polyfills.
• Split stream polyfills into StreamPolyfills (no System.Memory dependencies) and StreamMemoryPolyfills (Memory-based APIs) and updated consuming projects accordingly.
• Removed library-local polyfill sources (e.g., in System.IO.Hashing and System.Text.Encodings.Web) and updated call sites (e.g., System.Reflection.Metadata) to use BitOperations.PopCount.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Text.Json/src/System.Text.Json.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj	Replaces local BitOperations polyfill with shared BitOperationsPolyfills.
src/libraries/System.Text.Encodings.Web/src/Polyfills/System.Numerics.BitOperations.netstandard20.cs	Deletes library-local Log2 polyfill implementation.
src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs	Replaces BitArithmetic.CountBits usage with BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs	Switches external table count calculation to BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs	Removes CountBits helpers and uses BitOperations.PopCount for alignment asserts.
src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj	Adds shared BitOperationsPolyfills.cs for non-.NETCoreApp targets.
src/libraries/System.Net.ServerSentEvents/src/System.Net.ServerSentEvents.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj	Adds StreamPolyfills + StreamMemoryPolyfills for downlevel targets.
src/libraries/System.IO.Pipelines/src/System.IO.Pipelines.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/System.IO.Hashing/src/System/IO/Hashing/BitOperations.cs	Deletes library-local RotateLeft polyfill.
src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj	References shared BitOperationsPolyfills + StreamPolyfills for downlevel targets.
src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs	Adds Memory-based stream polyfills (ReadAsync, Write, WriteAsync) for downlevel TFMs.
src/libraries/Common/src/Polyfills/BitOperationsPolyfills.cs	Expands shared BitOperations polyfill surface (Log2/PopCount/rotates/TZC).

Comments suppressed due to low confidence (2)

src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:19

• In ReadAsync(Memory) the non-array-backed path rents a buffer from ArrayPool and then calls stream.ReadAsync(...) inline. If stream.ReadAsync throws synchronously (e.g., ObjectDisposedException / argument validation) before returning a Task, the rented buffer won't be returned and will leak. Consider capturing the Task inside a try/catch and returning the rented buffer in the catch before rethrowing, then passing the Task to FinishReadAsync for the awaited path.
  src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:76
• In WriteAsync(ReadOnlyMemory) the non-array-backed path rents a buffer and then calls stream.WriteAsync(...) without guarding against synchronous exceptions. If stream.WriteAsync throws before returning a Task, the rented buffer will leak because FinishWriteAsync won't run. Capture the Task in a try/catch (or use a try/finally around the call) and ensure ArrayPool.Return happens on the synchronous-throw path as well.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Fixed in commit d5a297c. Updated TagToTokenTests.cs to call BitOperations.PopCount(...) directly instead of the deleted BitArithmetic.CountBits(...), and added using System.Numerics;.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Already fixed in commit d5a297c — TagToTokenTests.cs now calls BitOperations.PopCount(...) directly with using System.Numerics;.

[Copilot]

Follow-up to #126287 — consolidates library-local BitOperations, Stream, StringBuilder, and Stack polyfills under src/libraries/Common/src/Polyfills/.

Changes

Common/src/Polyfills/BitOperationsPolyfills.cs

Expanded the existing polyfill (which only had RotateLeft(uint)) with the full set of methods needed by the affected libraries:

• Log2(uint) (deBruijn-based)
• PopCount(uint) and PopCount(ulong) (Hamming weight)
• RotateLeft(ulong), RotateRight(uint), RotateRight(ulong)
• TrailingZeroCount(uint) (deBruijn-based)

Common/src/Polyfills/StreamPolyfills.cs

Merged ReadExactly and CopyToAsync from the now-deleted Common/src/System/IO/StreamExtensions.netstandard.cs into the existing Stream polyfill (no System.Memory/System.Buffers dependencies).

Common/src/Polyfills/StreamMemoryPolyfills.cs(new)

New file containing the Memory-dependent stream extension methods, split out from StreamPolyfills.cs to avoid pulling in extra package references for consumers that don't need them:

• ReadAsync(Memory<byte>, CancellationToken)
• Write(ReadOnlyMemory<byte>)
• WriteAsync(ReadOnlyMemory<byte>, CancellationToken)

Common/src/Polyfills/StringBuilderPolyfills.cs(new)

Moved from Common/src/System/Text/StringBuilderExtensions.cs and reworked to follow polyfill directory conventions (file-scoped namespace, extension(StringBuilder) block syntax):

• Append(ReadOnlySpan<char>)

Common/src/Polyfills/StackPolyfills.cs(new)

Moved from System.Text.Json-local StackExtensions.netstandard.cs and reworked to follow polyfill directory conventions (file-scoped namespace, traditional extension methods matching DictionaryPolyfills.cs style for generic types):

• TryPeek<T>(out T)
• TryPop<T>(out T)

System.IO.Hashing

• Removed local System/IO/Hashing/BitOperations.cs (had RotateLeft(uint/ulong))
• Updated .csproj to reference BitOperationsPolyfills.cs and StreamPolyfills.cs

System.Text.Encodings.Web

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had Log2)
• Updated .csproj to reference BitOperationsPolyfills.cs

Microsoft.Bcl.Memory

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had TrailingZeroCount, RotateLeft(uint), RotateRight(uint))
• Updated .csproj to reference BitOperationsPolyfills.cs (which is a strict superset)

System.Reflection.Metadata

• Deleted BitArithmetic.CountBits(uint/ulong) wrapper methods; all call sites (BitArithmetic.Align, MetadataSizes, PEHeaderBuilder) now call BitOperations.PopCount directly
• Updated TagToTokenTests.cs to call BitOperations.PopCount directly (fixes .NETFramework build break)
• Added BitOperationsPolyfills.cs include in .csproj for non-.NETCoreApp targets

System.Net.ServerSentEvents, System.Text.Json, Microsoft.Extensions.Logging.Console, System.Net.Http.WinHttpHandler.Functional.Tests

• Reference StreamMemoryPolyfills.cs (use Memory-based stream methods)

System.IO.Pipelines

• References both StreamPolyfills.cs (uses CopyToAsync) and StreamMemoryPolyfills.cs (uses ReadAsync(Memory<byte>))

System.Speech, System.ServiceModel.Syndication

• Reference StreamPolyfills.cs only (use ReadExactly); no extra System.Memory/System.Buffers/System.Threading.Tasks.Extensions package references needed

System.Text.Json

• Updated .csproj to reference StringBuilderPolyfills.cs and StackPolyfills.cs from Common/src/Polyfills/
• Removed local StackExtensions.netstandard.cs

Note

This PR description was generated with the assistance of GitHub Copilot.

[github-actions]

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #126391

Holistic Assessment

Motivation: This PR continues the consolidation work from #126287 by merging three separate library-local BitOperations polyfill implementations (from System.IO.Hashing, System.Text.Encodings.Web, and System.Reflection.Metadata) into the shared Common/src/Polyfills/BitOperationsPolyfills.cs. The motivation is clear and well-justified — eliminating duplicate code and reducing #if NET/#else blocks.

Approach: The approach is consistent with the prior PR's pattern: centralize polyfills in a shared directory, update .csproj files to link the shared file for non-.NETCoreApp targets, and remove library-local copies. The consolidation is straightforward.

Summary: ❌ Needs Changes. The PR has a clear compilation error: a double opening brace in the Log2 method body that leaves the method unclosed. This must be fixed before merge. All other changes are clean.

---

Detailed Findings

❌ Compilation Error — Double { in Log2 method (BitOperationsPolyfills.cs:32-33)

Merge-blocking. The Log2 method has two consecutive opening braces but only one closing brace, creating an unbalanced brace structure that will not compile:

publicstaticintLog2(uintvalue){// ← line 32: method body opens{// ← line 33: extra block opens (from the old Log2SoftwareFallback body)// ... implementation ...returnUnsafe.AddByteOffset(...);}// ← line 50: closes only the inner block// method body is never closed!

Brace analysis of the entire file shows 8 opening braces vs 7 closing braces (final nesting depth 1, expected 0). All subsequent methods (PopCount, RotateLeft, etc.) end up syntactically nested inside the unclosed Log2 body, which is invalid C# — you cannot declare public static members inside a method.

Root cause: Commit 7ee20d2d ("Apply suggestion from @jkotas") merged the Log2 wrapper and Log2SoftwareFallback into a single method, but the replacement added a new opening brace for the merged method without removing the existing opening brace from the old Log2SoftwareFallback body.

Fix: Remove the extra { on line 33:

publicstaticintLog2(uintvalue){// No AggressiveInlining due to large method size// Has conventional contract 0->0 (Log(0) is undefined)

✅ Correctness — value | 1 removal in Log2 is safe

The original System.Text.Encodings.Web implementation called Log2SoftwareFallback(value | 1) to guarantee the 0 → 0 contract. The merged version drops the | 1. This is safe: when value = 0, the OR-shift cascade leaves it at 0, the DeBruijn multiplication yields index 0, and Log2DeBruijn[0] = 0, so Log2(0) correctly returns 0. All other inputs are unaffected since | 1 only set the lowest bit which doesn't change floor(log2(x)) for x > 0.

✅ Correctness — Algorithm implementations match BCL software fallbacks

The PopCount(uint), PopCount(ulong), TrailingZeroCount(uint), RotateLeft, and RotateRight implementations are faithful reproductions of the well-known software fallbacks (Hamming weight, DeBruijn sequences, bit rotations). These match the deleted library-local versions exactly.

✅ Consolidation — csproj changes are correct

All three .csproj files correctly:

• Replace local file references with $(CommonPath)Polyfills\BitOperationsPolyfills.cs
• Scope the reference to '$(TargetFrameworkIdentifier)' != '.NETCoreApp'
• Use appropriate Link attributes for solution explorer organization

✅ Cleanup — BitArithmetic.cs#if NET removal is clean

Removing the #if NET/#else blocks in System.Reflection.Metadata's BitArithmetic.cs and unconditionally calling BitOperations.PopCount is correct — the polyfill provides the same implementation that was previously inlined under #else.

💡 Observation — Extra methods not currently consumed

The consolidated polyfill includes RotateRight(uint/ulong) and TrailingZeroCount(uint) which are not currently used by any of the three consuming libraries. This is fine for a shared polyfill (other libraries may need them in the future), but worth noting for awareness.

Generated by Code Review for issue #126391 · ◷

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR consolidates downlevel (TargetFrameworkIdentifier != .NETCoreApp) polyfills for System.Numerics.BitOperations and Memory-based System.IO.Stream APIs into src/libraries/Common/src/Polyfills/, and updates multiple library projects to consume the shared implementations instead of library-local copies.

Changes:

• Expanded Common’s BitOperations polyfill and switched affected libraries to reference it from Common/src/Polyfills.
• Split stream polyfills into StreamPolyfills (no System.Memory dependencies) and StreamMemoryPolyfills (Memory-based APIs) and updated consuming projects accordingly.
• Removed library-local polyfill sources (e.g., in System.IO.Hashing and System.Text.Encodings.Web) and updated call sites (e.g., System.Reflection.Metadata) to use BitOperations.PopCount.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Text.Json/src/System.Text.Json.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj	Replaces local BitOperations polyfill with shared BitOperationsPolyfills.
src/libraries/System.Text.Encodings.Web/src/Polyfills/System.Numerics.BitOperations.netstandard20.cs	Deletes library-local Log2 polyfill implementation.
src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs	Replaces BitArithmetic.CountBits usage with BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs	Switches external table count calculation to BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs	Removes CountBits helpers and uses BitOperations.PopCount for alignment asserts.
src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj	Adds shared BitOperationsPolyfills.cs for non-.NETCoreApp targets.
src/libraries/System.Net.ServerSentEvents/src/System.Net.ServerSentEvents.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj	Adds StreamPolyfills + StreamMemoryPolyfills for downlevel targets.
src/libraries/System.IO.Pipelines/src/System.IO.Pipelines.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/System.IO.Hashing/src/System/IO/Hashing/BitOperations.cs	Deletes library-local RotateLeft polyfill.
src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj	References shared BitOperationsPolyfills + StreamPolyfills for downlevel targets.
src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs	Adds Memory-based stream polyfills (ReadAsync, Write, WriteAsync) for downlevel TFMs.
src/libraries/Common/src/Polyfills/BitOperationsPolyfills.cs	Expands shared BitOperations polyfill surface (Log2/PopCount/rotates/TZC).

Comments suppressed due to low confidence (2)

src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:19

• In ReadAsync(Memory) the non-array-backed path rents a buffer from ArrayPool and then calls stream.ReadAsync(...) inline. If stream.ReadAsync throws synchronously (e.g., ObjectDisposedException / argument validation) before returning a Task, the rented buffer won't be returned and will leak. Consider capturing the Task inside a try/catch and returning the rented buffer in the catch before rethrowing, then passing the Task to FinishReadAsync for the awaited path.
  src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:76
• In WriteAsync(ReadOnlyMemory) the non-array-backed path rents a buffer and then calls stream.WriteAsync(...) without guarding against synchronous exceptions. If stream.WriteAsync throws before returning a Task, the rented buffer will leak because FinishWriteAsync won't run. Capture the Task in a try/catch (or use a try/finally around the call) and ensure ArrayPool.Return happens on the synchronous-throw path as well.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Fixed in commit d5a297c. Updated TagToTokenTests.cs to call BitOperations.PopCount(...) directly instead of the deleted BitArithmetic.CountBits(...), and added using System.Numerics;.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Already fixed in commit d5a297c — TagToTokenTests.cs now calls BitOperations.PopCount(...) directly with using System.Numerics;.

[Copilot]

Follow-up to #126287 — consolidates library-local BitOperations, Stream, StringBuilder, and Stack polyfills under src/libraries/Common/src/Polyfills/.

Changes

Common/src/Polyfills/BitOperationsPolyfills.cs

Expanded the existing polyfill (which only had RotateLeft(uint)) with the full set of methods needed by the affected libraries:

• Log2(uint) (deBruijn-based)
• PopCount(uint) and PopCount(ulong) (Hamming weight)
• RotateLeft(ulong), RotateRight(uint), RotateRight(ulong)
• TrailingZeroCount(uint) (deBruijn-based)

Common/src/Polyfills/StreamPolyfills.cs

Merged ReadExactly and CopyToAsync from the now-deleted Common/src/System/IO/StreamExtensions.netstandard.cs into the existing Stream polyfill (no System.Memory/System.Buffers dependencies).

Common/src/Polyfills/StreamMemoryPolyfills.cs(new)

New file containing the Memory-dependent stream extension methods, split out from StreamPolyfills.cs to avoid pulling in extra package references for consumers that don't need them:

• ReadAsync(Memory<byte>, CancellationToken)
• Write(ReadOnlyMemory<byte>)
• WriteAsync(ReadOnlyMemory<byte>, CancellationToken)

Common/src/Polyfills/StringBuilderPolyfills.cs(new)

Moved from Common/src/System/Text/StringBuilderExtensions.cs and reworked to follow polyfill directory conventions (file-scoped namespace, extension(StringBuilder) block syntax):

• Append(ReadOnlySpan<char>)

Common/src/Polyfills/StackPolyfills.cs(new)

Moved from System.Text.Json-local StackExtensions.netstandard.cs and reworked to follow polyfill directory conventions (file-scoped namespace, traditional extension methods matching DictionaryPolyfills.cs style for generic types):

• TryPeek<T>(out T)
• TryPop<T>(out T)

System.IO.Hashing

• Removed local System/IO/Hashing/BitOperations.cs (had RotateLeft(uint/ulong))
• Updated .csproj to reference BitOperationsPolyfills.cs and StreamPolyfills.cs

System.Text.Encodings.Web

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had Log2)
• Updated .csproj to reference BitOperationsPolyfills.cs

Microsoft.Bcl.Memory

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had TrailingZeroCount, RotateLeft(uint), RotateRight(uint))
• Updated .csproj to reference BitOperationsPolyfills.cs (which is a strict superset)

System.Reflection.Metadata

• Deleted BitArithmetic.CountBits(uint/ulong) wrapper methods; all call sites (BitArithmetic.Align, MetadataSizes, PEHeaderBuilder) now call BitOperations.PopCount directly
• Updated TagToTokenTests.cs to call BitOperations.PopCount directly (fixes .NETFramework build break)
• Added BitOperationsPolyfills.cs include in .csproj for non-.NETCoreApp targets

System.Net.ServerSentEvents, System.Text.Json, Microsoft.Extensions.Logging.Console, System.Net.Http.WinHttpHandler.Functional.Tests

• Reference StreamMemoryPolyfills.cs (use Memory-based stream methods)

System.IO.Pipelines

• References both StreamPolyfills.cs (uses CopyToAsync) and StreamMemoryPolyfills.cs (uses ReadAsync(Memory<byte>))

System.Speech, System.ServiceModel.Syndication

• Reference StreamPolyfills.cs only (use ReadExactly); no extra System.Memory/System.Buffers/System.Threading.Tasks.Extensions package references needed

System.Text.Json

• Updated .csproj to reference StringBuilderPolyfills.cs and StackPolyfills.cs from Common/src/Polyfills/
• Removed local StackExtensions.netstandard.cs

Note

This PR description was generated with the assistance of GitHub Copilot.

[github-actions]

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #126391

Holistic Assessment

Motivation: This PR continues the consolidation work from #126287 by merging three separate library-local BitOperations polyfill implementations (from System.IO.Hashing, System.Text.Encodings.Web, and System.Reflection.Metadata) into the shared Common/src/Polyfills/BitOperationsPolyfills.cs. The motivation is clear and well-justified — eliminating duplicate code and reducing #if NET/#else blocks.

Approach: The approach is consistent with the prior PR's pattern: centralize polyfills in a shared directory, update .csproj files to link the shared file for non-.NETCoreApp targets, and remove library-local copies. The consolidation is straightforward.

Summary: ❌ Needs Changes. The PR has a clear compilation error: a double opening brace in the Log2 method body that leaves the method unclosed. This must be fixed before merge. All other changes are clean.

---

Detailed Findings

❌ Compilation Error — Double { in Log2 method (BitOperationsPolyfills.cs:32-33)

Merge-blocking. The Log2 method has two consecutive opening braces but only one closing brace, creating an unbalanced brace structure that will not compile:

publicstaticintLog2(uintvalue){// ← line 32: method body opens{// ← line 33: extra block opens (from the old Log2SoftwareFallback body)// ... implementation ...returnUnsafe.AddByteOffset(...);}// ← line 50: closes only the inner block// method body is never closed!

Brace analysis of the entire file shows 8 opening braces vs 7 closing braces (final nesting depth 1, expected 0). All subsequent methods (PopCount, RotateLeft, etc.) end up syntactically nested inside the unclosed Log2 body, which is invalid C# — you cannot declare public static members inside a method.

Root cause: Commit 7ee20d2d ("Apply suggestion from @jkotas") merged the Log2 wrapper and Log2SoftwareFallback into a single method, but the replacement added a new opening brace for the merged method without removing the existing opening brace from the old Log2SoftwareFallback body.

Fix: Remove the extra { on line 33:

publicstaticintLog2(uintvalue){// No AggressiveInlining due to large method size// Has conventional contract 0->0 (Log(0) is undefined)

✅ Correctness — value | 1 removal in Log2 is safe

The original System.Text.Encodings.Web implementation called Log2SoftwareFallback(value | 1) to guarantee the 0 → 0 contract. The merged version drops the | 1. This is safe: when value = 0, the OR-shift cascade leaves it at 0, the DeBruijn multiplication yields index 0, and Log2DeBruijn[0] = 0, so Log2(0) correctly returns 0. All other inputs are unaffected since | 1 only set the lowest bit which doesn't change floor(log2(x)) for x > 0.

✅ Correctness — Algorithm implementations match BCL software fallbacks

The PopCount(uint), PopCount(ulong), TrailingZeroCount(uint), RotateLeft, and RotateRight implementations are faithful reproductions of the well-known software fallbacks (Hamming weight, DeBruijn sequences, bit rotations). These match the deleted library-local versions exactly.

✅ Consolidation — csproj changes are correct

All three .csproj files correctly:

• Replace local file references with $(CommonPath)Polyfills\BitOperationsPolyfills.cs
• Scope the reference to '$(TargetFrameworkIdentifier)' != '.NETCoreApp'
• Use appropriate Link attributes for solution explorer organization

✅ Cleanup — BitArithmetic.cs#if NET removal is clean

Removing the #if NET/#else blocks in System.Reflection.Metadata's BitArithmetic.cs and unconditionally calling BitOperations.PopCount is correct — the polyfill provides the same implementation that was previously inlined under #else.

💡 Observation — Extra methods not currently consumed

The consolidated polyfill includes RotateRight(uint/ulong) and TrailingZeroCount(uint) which are not currently used by any of the three consuming libraries. This is fine for a shared polyfill (other libraries may need them in the future), but worth noting for awareness.

Generated by Code Review for issue #126391 · ◷

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR consolidates downlevel (TargetFrameworkIdentifier != .NETCoreApp) polyfills for System.Numerics.BitOperations and Memory-based System.IO.Stream APIs into src/libraries/Common/src/Polyfills/, and updates multiple library projects to consume the shared implementations instead of library-local copies.

Changes:

• Expanded Common’s BitOperations polyfill and switched affected libraries to reference it from Common/src/Polyfills.
• Split stream polyfills into StreamPolyfills (no System.Memory dependencies) and StreamMemoryPolyfills (Memory-based APIs) and updated consuming projects accordingly.
• Removed library-local polyfill sources (e.g., in System.IO.Hashing and System.Text.Encodings.Web) and updated call sites (e.g., System.Reflection.Metadata) to use BitOperations.PopCount.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Text.Json/src/System.Text.Json.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj	Replaces local BitOperations polyfill with shared BitOperationsPolyfills.
src/libraries/System.Text.Encodings.Web/src/Polyfills/System.Numerics.BitOperations.netstandard20.cs	Deletes library-local Log2 polyfill implementation.
src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs	Replaces BitArithmetic.CountBits usage with BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs	Switches external table count calculation to BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs	Removes CountBits helpers and uses BitOperations.PopCount for alignment asserts.
src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj	Adds shared BitOperationsPolyfills.cs for non-.NETCoreApp targets.
src/libraries/System.Net.ServerSentEvents/src/System.Net.ServerSentEvents.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj	Adds StreamPolyfills + StreamMemoryPolyfills for downlevel targets.
src/libraries/System.IO.Pipelines/src/System.IO.Pipelines.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/System.IO.Hashing/src/System/IO/Hashing/BitOperations.cs	Deletes library-local RotateLeft polyfill.
src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj	References shared BitOperationsPolyfills + StreamPolyfills for downlevel targets.
src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs	Adds Memory-based stream polyfills (ReadAsync, Write, WriteAsync) for downlevel TFMs.
src/libraries/Common/src/Polyfills/BitOperationsPolyfills.cs	Expands shared BitOperations polyfill surface (Log2/PopCount/rotates/TZC).

Comments suppressed due to low confidence (2)

src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:19

• In ReadAsync(Memory) the non-array-backed path rents a buffer from ArrayPool and then calls stream.ReadAsync(...) inline. If stream.ReadAsync throws synchronously (e.g., ObjectDisposedException / argument validation) before returning a Task, the rented buffer won't be returned and will leak. Consider capturing the Task inside a try/catch and returning the rented buffer in the catch before rethrowing, then passing the Task to FinishReadAsync for the awaited path.
  src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:76
• In WriteAsync(ReadOnlyMemory) the non-array-backed path rents a buffer and then calls stream.WriteAsync(...) without guarding against synchronous exceptions. If stream.WriteAsync throws before returning a Task, the rented buffer will leak because FinishWriteAsync won't run. Capture the Task in a try/catch (or use a try/finally around the call) and ensure ArrayPool.Return happens on the synchronous-throw path as well.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Fixed in commit d5a297c. Updated TagToTokenTests.cs to call BitOperations.PopCount(...) directly instead of the deleted BitArithmetic.CountBits(...), and added using System.Numerics;.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Already fixed in commit d5a297c — TagToTokenTests.cs now calls BitOperations.PopCount(...) directly with using System.Numerics;.

[Copilot]

Follow-up to #126287 — consolidates library-local BitOperations, Stream, StringBuilder, and Stack polyfills under src/libraries/Common/src/Polyfills/.

Changes

Common/src/Polyfills/BitOperationsPolyfills.cs

Expanded the existing polyfill (which only had RotateLeft(uint)) with the full set of methods needed by the affected libraries:

• Log2(uint) (deBruijn-based)
• PopCount(uint) and PopCount(ulong) (Hamming weight)
• RotateLeft(ulong), RotateRight(uint), RotateRight(ulong)
• TrailingZeroCount(uint) (deBruijn-based)

Common/src/Polyfills/StreamPolyfills.cs

Merged ReadExactly and CopyToAsync from the now-deleted Common/src/System/IO/StreamExtensions.netstandard.cs into the existing Stream polyfill (no System.Memory/System.Buffers dependencies).

Common/src/Polyfills/StreamMemoryPolyfills.cs(new)

New file containing the Memory-dependent stream extension methods, split out from StreamPolyfills.cs to avoid pulling in extra package references for consumers that don't need them:

• ReadAsync(Memory<byte>, CancellationToken)
• Write(ReadOnlyMemory<byte>)
• WriteAsync(ReadOnlyMemory<byte>, CancellationToken)

Common/src/Polyfills/StringBuilderPolyfills.cs(new)

Moved from Common/src/System/Text/StringBuilderExtensions.cs and reworked to follow polyfill directory conventions (file-scoped namespace, extension(StringBuilder) block syntax):

• Append(ReadOnlySpan<char>)

Common/src/Polyfills/StackPolyfills.cs(new)

Moved from System.Text.Json-local StackExtensions.netstandard.cs and reworked to follow polyfill directory conventions (file-scoped namespace, traditional extension methods matching DictionaryPolyfills.cs style for generic types):

• TryPeek<T>(out T)
• TryPop<T>(out T)

System.IO.Hashing

• Removed local System/IO/Hashing/BitOperations.cs (had RotateLeft(uint/ulong))
• Updated .csproj to reference BitOperationsPolyfills.cs and StreamPolyfills.cs

System.Text.Encodings.Web

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had Log2)
• Updated .csproj to reference BitOperationsPolyfills.cs

Microsoft.Bcl.Memory

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had TrailingZeroCount, RotateLeft(uint), RotateRight(uint))
• Updated .csproj to reference BitOperationsPolyfills.cs (which is a strict superset)

System.Reflection.Metadata

• Deleted BitArithmetic.CountBits(uint/ulong) wrapper methods; all call sites (BitArithmetic.Align, MetadataSizes, PEHeaderBuilder) now call BitOperations.PopCount directly
• Updated TagToTokenTests.cs to call BitOperations.PopCount directly (fixes .NETFramework build break)
• Added BitOperationsPolyfills.cs include in .csproj for non-.NETCoreApp targets

System.Net.ServerSentEvents, System.Text.Json, Microsoft.Extensions.Logging.Console, System.Net.Http.WinHttpHandler.Functional.Tests

• Reference StreamMemoryPolyfills.cs (use Memory-based stream methods)

System.IO.Pipelines

• References both StreamPolyfills.cs (uses CopyToAsync) and StreamMemoryPolyfills.cs (uses ReadAsync(Memory<byte>))

System.Speech, System.ServiceModel.Syndication

• Reference StreamPolyfills.cs only (use ReadExactly); no extra System.Memory/System.Buffers/System.Threading.Tasks.Extensions package references needed

System.Text.Json

• Updated .csproj to reference StringBuilderPolyfills.cs and StackPolyfills.cs from Common/src/Polyfills/
• Removed local StackExtensions.netstandard.cs

Note

This PR description was generated with the assistance of GitHub Copilot.

[github-actions]

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #126391

Holistic Assessment

Motivation: This PR continues the consolidation work from #126287 by merging three separate library-local BitOperations polyfill implementations (from System.IO.Hashing, System.Text.Encodings.Web, and System.Reflection.Metadata) into the shared Common/src/Polyfills/BitOperationsPolyfills.cs. The motivation is clear and well-justified — eliminating duplicate code and reducing #if NET/#else blocks.

Approach: The approach is consistent with the prior PR's pattern: centralize polyfills in a shared directory, update .csproj files to link the shared file for non-.NETCoreApp targets, and remove library-local copies. The consolidation is straightforward.

Summary: ❌ Needs Changes. The PR has a clear compilation error: a double opening brace in the Log2 method body that leaves the method unclosed. This must be fixed before merge. All other changes are clean.

---

Detailed Findings

❌ Compilation Error — Double { in Log2 method (BitOperationsPolyfills.cs:32-33)

Merge-blocking. The Log2 method has two consecutive opening braces but only one closing brace, creating an unbalanced brace structure that will not compile:

publicstaticintLog2(uintvalue){// ← line 32: method body opens{// ← line 33: extra block opens (from the old Log2SoftwareFallback body)// ... implementation ...returnUnsafe.AddByteOffset(...);}// ← line 50: closes only the inner block// method body is never closed!

Brace analysis of the entire file shows 8 opening braces vs 7 closing braces (final nesting depth 1, expected 0). All subsequent methods (PopCount, RotateLeft, etc.) end up syntactically nested inside the unclosed Log2 body, which is invalid C# — you cannot declare public static members inside a method.

Root cause: Commit 7ee20d2d ("Apply suggestion from @jkotas") merged the Log2 wrapper and Log2SoftwareFallback into a single method, but the replacement added a new opening brace for the merged method without removing the existing opening brace from the old Log2SoftwareFallback body.

Fix: Remove the extra { on line 33:

publicstaticintLog2(uintvalue){// No AggressiveInlining due to large method size// Has conventional contract 0->0 (Log(0) is undefined)

✅ Correctness — value | 1 removal in Log2 is safe

The original System.Text.Encodings.Web implementation called Log2SoftwareFallback(value | 1) to guarantee the 0 → 0 contract. The merged version drops the | 1. This is safe: when value = 0, the OR-shift cascade leaves it at 0, the DeBruijn multiplication yields index 0, and Log2DeBruijn[0] = 0, so Log2(0) correctly returns 0. All other inputs are unaffected since | 1 only set the lowest bit which doesn't change floor(log2(x)) for x > 0.

✅ Correctness — Algorithm implementations match BCL software fallbacks

The PopCount(uint), PopCount(ulong), TrailingZeroCount(uint), RotateLeft, and RotateRight implementations are faithful reproductions of the well-known software fallbacks (Hamming weight, DeBruijn sequences, bit rotations). These match the deleted library-local versions exactly.

✅ Consolidation — csproj changes are correct

All three .csproj files correctly:

• Replace local file references with $(CommonPath)Polyfills\BitOperationsPolyfills.cs
• Scope the reference to '$(TargetFrameworkIdentifier)' != '.NETCoreApp'
• Use appropriate Link attributes for solution explorer organization

✅ Cleanup — BitArithmetic.cs#if NET removal is clean

Removing the #if NET/#else blocks in System.Reflection.Metadata's BitArithmetic.cs and unconditionally calling BitOperations.PopCount is correct — the polyfill provides the same implementation that was previously inlined under #else.

💡 Observation — Extra methods not currently consumed

The consolidated polyfill includes RotateRight(uint/ulong) and TrailingZeroCount(uint) which are not currently used by any of the three consuming libraries. This is fine for a shared polyfill (other libraries may need them in the future), but worth noting for awareness.

Generated by Code Review for issue #126391 · ◷

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR consolidates downlevel (TargetFrameworkIdentifier != .NETCoreApp) polyfills for System.Numerics.BitOperations and Memory-based System.IO.Stream APIs into src/libraries/Common/src/Polyfills/, and updates multiple library projects to consume the shared implementations instead of library-local copies.

Changes:

• Expanded Common’s BitOperations polyfill and switched affected libraries to reference it from Common/src/Polyfills.
• Split stream polyfills into StreamPolyfills (no System.Memory dependencies) and StreamMemoryPolyfills (Memory-based APIs) and updated consuming projects accordingly.
• Removed library-local polyfill sources (e.g., in System.IO.Hashing and System.Text.Encodings.Web) and updated call sites (e.g., System.Reflection.Metadata) to use BitOperations.PopCount.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Text.Json/src/System.Text.Json.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj	Replaces local BitOperations polyfill with shared BitOperationsPolyfills.
src/libraries/System.Text.Encodings.Web/src/Polyfills/System.Numerics.BitOperations.netstandard20.cs	Deletes library-local Log2 polyfill implementation.
src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs	Replaces BitArithmetic.CountBits usage with BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs	Switches external table count calculation to BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs	Removes CountBits helpers and uses BitOperations.PopCount for alignment asserts.
src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj	Adds shared BitOperationsPolyfills.cs for non-.NETCoreApp targets.
src/libraries/System.Net.ServerSentEvents/src/System.Net.ServerSentEvents.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj	Adds StreamPolyfills + StreamMemoryPolyfills for downlevel targets.
src/libraries/System.IO.Pipelines/src/System.IO.Pipelines.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/System.IO.Hashing/src/System/IO/Hashing/BitOperations.cs	Deletes library-local RotateLeft polyfill.
src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj	References shared BitOperationsPolyfills + StreamPolyfills for downlevel targets.
src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs	Adds Memory-based stream polyfills (ReadAsync, Write, WriteAsync) for downlevel TFMs.
src/libraries/Common/src/Polyfills/BitOperationsPolyfills.cs	Expands shared BitOperations polyfill surface (Log2/PopCount/rotates/TZC).

Comments suppressed due to low confidence (2)

src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:19

• In ReadAsync(Memory) the non-array-backed path rents a buffer from ArrayPool and then calls stream.ReadAsync(...) inline. If stream.ReadAsync throws synchronously (e.g., ObjectDisposedException / argument validation) before returning a Task, the rented buffer won't be returned and will leak. Consider capturing the Task inside a try/catch and returning the rented buffer in the catch before rethrowing, then passing the Task to FinishReadAsync for the awaited path.
  src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:76
• In WriteAsync(ReadOnlyMemory) the non-array-backed path rents a buffer and then calls stream.WriteAsync(...) without guarding against synchronous exceptions. If stream.WriteAsync throws before returning a Task, the rented buffer will leak because FinishWriteAsync won't run. Capture the Task in a try/catch (or use a try/finally around the call) and ensure ArrayPool.Return happens on the synchronous-throw path as well.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Fixed in commit d5a297c. Updated TagToTokenTests.cs to call BitOperations.PopCount(...) directly instead of the deleted BitArithmetic.CountBits(...), and added using System.Numerics;.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Already fixed in commit d5a297c — TagToTokenTests.cs now calls BitOperations.PopCount(...) directly with using System.Numerics;.

[Copilot]

Follow-up to #126287 — consolidates library-local BitOperations, Stream, StringBuilder, and Stack polyfills under src/libraries/Common/src/Polyfills/.

Changes

Common/src/Polyfills/BitOperationsPolyfills.cs

Expanded the existing polyfill (which only had RotateLeft(uint)) with the full set of methods needed by the affected libraries:

• Log2(uint) (deBruijn-based)
• PopCount(uint) and PopCount(ulong) (Hamming weight)
• RotateLeft(ulong), RotateRight(uint), RotateRight(ulong)
• TrailingZeroCount(uint) (deBruijn-based)

Common/src/Polyfills/StreamPolyfills.cs

Merged ReadExactly and CopyToAsync from the now-deleted Common/src/System/IO/StreamExtensions.netstandard.cs into the existing Stream polyfill (no System.Memory/System.Buffers dependencies).

Common/src/Polyfills/StreamMemoryPolyfills.cs(new)

New file containing the Memory-dependent stream extension methods, split out from StreamPolyfills.cs to avoid pulling in extra package references for consumers that don't need them:

• ReadAsync(Memory<byte>, CancellationToken)
• Write(ReadOnlyMemory<byte>)
• WriteAsync(ReadOnlyMemory<byte>, CancellationToken)

Common/src/Polyfills/StringBuilderPolyfills.cs(new)

Moved from Common/src/System/Text/StringBuilderExtensions.cs and reworked to follow polyfill directory conventions (file-scoped namespace, extension(StringBuilder) block syntax):

• Append(ReadOnlySpan<char>)

Common/src/Polyfills/StackPolyfills.cs(new)

Moved from System.Text.Json-local StackExtensions.netstandard.cs and reworked to follow polyfill directory conventions (file-scoped namespace, traditional extension methods matching DictionaryPolyfills.cs style for generic types):

• TryPeek<T>(out T)
• TryPop<T>(out T)

System.IO.Hashing

• Removed local System/IO/Hashing/BitOperations.cs (had RotateLeft(uint/ulong))
• Updated .csproj to reference BitOperationsPolyfills.cs and StreamPolyfills.cs

System.Text.Encodings.Web

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had Log2)
• Updated .csproj to reference BitOperationsPolyfills.cs

Microsoft.Bcl.Memory

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had TrailingZeroCount, RotateLeft(uint), RotateRight(uint))
• Updated .csproj to reference BitOperationsPolyfills.cs (which is a strict superset)

System.Reflection.Metadata

• Deleted BitArithmetic.CountBits(uint/ulong) wrapper methods; all call sites (BitArithmetic.Align, MetadataSizes, PEHeaderBuilder) now call BitOperations.PopCount directly
• Updated TagToTokenTests.cs to call BitOperations.PopCount directly (fixes .NETFramework build break)
• Added BitOperationsPolyfills.cs include in .csproj for non-.NETCoreApp targets

System.Net.ServerSentEvents, System.Text.Json, Microsoft.Extensions.Logging.Console, System.Net.Http.WinHttpHandler.Functional.Tests

• Reference StreamMemoryPolyfills.cs (use Memory-based stream methods)

System.IO.Pipelines

• References both StreamPolyfills.cs (uses CopyToAsync) and StreamMemoryPolyfills.cs (uses ReadAsync(Memory<byte>))

System.Speech, System.ServiceModel.Syndication

• Reference StreamPolyfills.cs only (use ReadExactly); no extra System.Memory/System.Buffers/System.Threading.Tasks.Extensions package references needed

System.Text.Json

• Updated .csproj to reference StringBuilderPolyfills.cs and StackPolyfills.cs from Common/src/Polyfills/
• Removed local StackExtensions.netstandard.cs

Note

This PR description was generated with the assistance of GitHub Copilot.

[github-actions]

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #126391

Holistic Assessment

Motivation: This PR continues the consolidation work from #126287 by merging three separate library-local BitOperations polyfill implementations (from System.IO.Hashing, System.Text.Encodings.Web, and System.Reflection.Metadata) into the shared Common/src/Polyfills/BitOperationsPolyfills.cs. The motivation is clear and well-justified — eliminating duplicate code and reducing #if NET/#else blocks.

Approach: The approach is consistent with the prior PR's pattern: centralize polyfills in a shared directory, update .csproj files to link the shared file for non-.NETCoreApp targets, and remove library-local copies. The consolidation is straightforward.

Summary: ❌ Needs Changes. The PR has a clear compilation error: a double opening brace in the Log2 method body that leaves the method unclosed. This must be fixed before merge. All other changes are clean.

---

Detailed Findings

❌ Compilation Error — Double { in Log2 method (BitOperationsPolyfills.cs:32-33)

Merge-blocking. The Log2 method has two consecutive opening braces but only one closing brace, creating an unbalanced brace structure that will not compile:

publicstaticintLog2(uintvalue){// ← line 32: method body opens{// ← line 33: extra block opens (from the old Log2SoftwareFallback body)// ... implementation ...returnUnsafe.AddByteOffset(...);}// ← line 50: closes only the inner block// method body is never closed!

Brace analysis of the entire file shows 8 opening braces vs 7 closing braces (final nesting depth 1, expected 0). All subsequent methods (PopCount, RotateLeft, etc.) end up syntactically nested inside the unclosed Log2 body, which is invalid C# — you cannot declare public static members inside a method.

Root cause: Commit 7ee20d2d ("Apply suggestion from @jkotas") merged the Log2 wrapper and Log2SoftwareFallback into a single method, but the replacement added a new opening brace for the merged method without removing the existing opening brace from the old Log2SoftwareFallback body.

Fix: Remove the extra { on line 33:

publicstaticintLog2(uintvalue){// No AggressiveInlining due to large method size// Has conventional contract 0->0 (Log(0) is undefined)

✅ Correctness — value | 1 removal in Log2 is safe

The original System.Text.Encodings.Web implementation called Log2SoftwareFallback(value | 1) to guarantee the 0 → 0 contract. The merged version drops the | 1. This is safe: when value = 0, the OR-shift cascade leaves it at 0, the DeBruijn multiplication yields index 0, and Log2DeBruijn[0] = 0, so Log2(0) correctly returns 0. All other inputs are unaffected since | 1 only set the lowest bit which doesn't change floor(log2(x)) for x > 0.

✅ Correctness — Algorithm implementations match BCL software fallbacks

The PopCount(uint), PopCount(ulong), TrailingZeroCount(uint), RotateLeft, and RotateRight implementations are faithful reproductions of the well-known software fallbacks (Hamming weight, DeBruijn sequences, bit rotations). These match the deleted library-local versions exactly.

✅ Consolidation — csproj changes are correct

All three .csproj files correctly:

• Replace local file references with $(CommonPath)Polyfills\BitOperationsPolyfills.cs
• Scope the reference to '$(TargetFrameworkIdentifier)' != '.NETCoreApp'
• Use appropriate Link attributes for solution explorer organization

✅ Cleanup — BitArithmetic.cs#if NET removal is clean

Removing the #if NET/#else blocks in System.Reflection.Metadata's BitArithmetic.cs and unconditionally calling BitOperations.PopCount is correct — the polyfill provides the same implementation that was previously inlined under #else.

💡 Observation — Extra methods not currently consumed

The consolidated polyfill includes RotateRight(uint/ulong) and TrailingZeroCount(uint) which are not currently used by any of the three consuming libraries. This is fine for a shared polyfill (other libraries may need them in the future), but worth noting for awareness.

Generated by Code Review for issue #126391 · ◷

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR consolidates downlevel (TargetFrameworkIdentifier != .NETCoreApp) polyfills for System.Numerics.BitOperations and Memory-based System.IO.Stream APIs into src/libraries/Common/src/Polyfills/, and updates multiple library projects to consume the shared implementations instead of library-local copies.

Changes:

• Expanded Common’s BitOperations polyfill and switched affected libraries to reference it from Common/src/Polyfills.
• Split stream polyfills into StreamPolyfills (no System.Memory dependencies) and StreamMemoryPolyfills (Memory-based APIs) and updated consuming projects accordingly.
• Removed library-local polyfill sources (e.g., in System.IO.Hashing and System.Text.Encodings.Web) and updated call sites (e.g., System.Reflection.Metadata) to use BitOperations.PopCount.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Text.Json/src/System.Text.Json.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj	Replaces local BitOperations polyfill with shared BitOperationsPolyfills.
src/libraries/System.Text.Encodings.Web/src/Polyfills/System.Numerics.BitOperations.netstandard20.cs	Deletes library-local Log2 polyfill implementation.
src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs	Replaces BitArithmetic.CountBits usage with BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs	Switches external table count calculation to BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs	Removes CountBits helpers and uses BitOperations.PopCount for alignment asserts.
src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj	Adds shared BitOperationsPolyfills.cs for non-.NETCoreApp targets.
src/libraries/System.Net.ServerSentEvents/src/System.Net.ServerSentEvents.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj	Adds StreamPolyfills + StreamMemoryPolyfills for downlevel targets.
src/libraries/System.IO.Pipelines/src/System.IO.Pipelines.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/System.IO.Hashing/src/System/IO/Hashing/BitOperations.cs	Deletes library-local RotateLeft polyfill.
src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj	References shared BitOperationsPolyfills + StreamPolyfills for downlevel targets.
src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs	Adds Memory-based stream polyfills (ReadAsync, Write, WriteAsync) for downlevel TFMs.
src/libraries/Common/src/Polyfills/BitOperationsPolyfills.cs	Expands shared BitOperations polyfill surface (Log2/PopCount/rotates/TZC).

Comments suppressed due to low confidence (2)

src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:19

• In ReadAsync(Memory) the non-array-backed path rents a buffer from ArrayPool and then calls stream.ReadAsync(...) inline. If stream.ReadAsync throws synchronously (e.g., ObjectDisposedException / argument validation) before returning a Task, the rented buffer won't be returned and will leak. Consider capturing the Task inside a try/catch and returning the rented buffer in the catch before rethrowing, then passing the Task to FinishReadAsync for the awaited path.
  src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:76
• In WriteAsync(ReadOnlyMemory) the non-array-backed path rents a buffer and then calls stream.WriteAsync(...) without guarding against synchronous exceptions. If stream.WriteAsync throws before returning a Task, the rented buffer will leak because FinishWriteAsync won't run. Capture the Task in a try/catch (or use a try/finally around the call) and ensure ArrayPool.Return happens on the synchronous-throw path as well.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Fixed in commit d5a297c. Updated TagToTokenTests.cs to call BitOperations.PopCount(...) directly instead of the deleted BitArithmetic.CountBits(...), and added using System.Numerics;.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Already fixed in commit d5a297c — TagToTokenTests.cs now calls BitOperations.PopCount(...) directly with using System.Numerics;.

[Copilot]

Follow-up to #126287 — consolidates library-local BitOperations, Stream, StringBuilder, and Stack polyfills under src/libraries/Common/src/Polyfills/.

Changes

Common/src/Polyfills/BitOperationsPolyfills.cs

Expanded the existing polyfill (which only had RotateLeft(uint)) with the full set of methods needed by the affected libraries:

• Log2(uint) (deBruijn-based)
• PopCount(uint) and PopCount(ulong) (Hamming weight)
• RotateLeft(ulong), RotateRight(uint), RotateRight(ulong)
• TrailingZeroCount(uint) (deBruijn-based)

Common/src/Polyfills/StreamPolyfills.cs

Merged ReadExactly and CopyToAsync from the now-deleted Common/src/System/IO/StreamExtensions.netstandard.cs into the existing Stream polyfill (no System.Memory/System.Buffers dependencies).

Common/src/Polyfills/StreamMemoryPolyfills.cs(new)

New file containing the Memory-dependent stream extension methods, split out from StreamPolyfills.cs to avoid pulling in extra package references for consumers that don't need them:

• ReadAsync(Memory<byte>, CancellationToken)
• Write(ReadOnlyMemory<byte>)
• WriteAsync(ReadOnlyMemory<byte>, CancellationToken)

Common/src/Polyfills/StringBuilderPolyfills.cs(new)

Moved from Common/src/System/Text/StringBuilderExtensions.cs and reworked to follow polyfill directory conventions (file-scoped namespace, extension(StringBuilder) block syntax):

• Append(ReadOnlySpan<char>)

Common/src/Polyfills/StackPolyfills.cs(new)

Moved from System.Text.Json-local StackExtensions.netstandard.cs and reworked to follow polyfill directory conventions (file-scoped namespace, traditional extension methods matching DictionaryPolyfills.cs style for generic types):

• TryPeek<T>(out T)
• TryPop<T>(out T)

System.IO.Hashing

• Removed local System/IO/Hashing/BitOperations.cs (had RotateLeft(uint/ulong))
• Updated .csproj to reference BitOperationsPolyfills.cs and StreamPolyfills.cs

System.Text.Encodings.Web

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had Log2)
• Updated .csproj to reference BitOperationsPolyfills.cs

Microsoft.Bcl.Memory

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had TrailingZeroCount, RotateLeft(uint), RotateRight(uint))
• Updated .csproj to reference BitOperationsPolyfills.cs (which is a strict superset)

System.Reflection.Metadata

• Deleted BitArithmetic.CountBits(uint/ulong) wrapper methods; all call sites (BitArithmetic.Align, MetadataSizes, PEHeaderBuilder) now call BitOperations.PopCount directly
• Updated TagToTokenTests.cs to call BitOperations.PopCount directly (fixes .NETFramework build break)
• Added BitOperationsPolyfills.cs include in .csproj for non-.NETCoreApp targets

System.Net.ServerSentEvents, System.Text.Json, Microsoft.Extensions.Logging.Console, System.Net.Http.WinHttpHandler.Functional.Tests

• Reference StreamMemoryPolyfills.cs (use Memory-based stream methods)

System.IO.Pipelines

• References both StreamPolyfills.cs (uses CopyToAsync) and StreamMemoryPolyfills.cs (uses ReadAsync(Memory<byte>))

System.Speech, System.ServiceModel.Syndication

• Reference StreamPolyfills.cs only (use ReadExactly); no extra System.Memory/System.Buffers/System.Threading.Tasks.Extensions package references needed

System.Text.Json

• Updated .csproj to reference StringBuilderPolyfills.cs and StackPolyfills.cs from Common/src/Polyfills/
• Removed local StackExtensions.netstandard.cs

Note

This PR description was generated with the assistance of GitHub Copilot.

[github-actions]

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #126391

Holistic Assessment

Motivation: This PR continues the consolidation work from #126287 by merging three separate library-local BitOperations polyfill implementations (from System.IO.Hashing, System.Text.Encodings.Web, and System.Reflection.Metadata) into the shared Common/src/Polyfills/BitOperationsPolyfills.cs. The motivation is clear and well-justified — eliminating duplicate code and reducing #if NET/#else blocks.

Approach: The approach is consistent with the prior PR's pattern: centralize polyfills in a shared directory, update .csproj files to link the shared file for non-.NETCoreApp targets, and remove library-local copies. The consolidation is straightforward.

Summary: ❌ Needs Changes. The PR has a clear compilation error: a double opening brace in the Log2 method body that leaves the method unclosed. This must be fixed before merge. All other changes are clean.

---

Detailed Findings

❌ Compilation Error — Double { in Log2 method (BitOperationsPolyfills.cs:32-33)

Merge-blocking. The Log2 method has two consecutive opening braces but only one closing brace, creating an unbalanced brace structure that will not compile:

publicstaticintLog2(uintvalue){// ← line 32: method body opens{// ← line 33: extra block opens (from the old Log2SoftwareFallback body)// ... implementation ...returnUnsafe.AddByteOffset(...);}// ← line 50: closes only the inner block// method body is never closed!

Brace analysis of the entire file shows 8 opening braces vs 7 closing braces (final nesting depth 1, expected 0). All subsequent methods (PopCount, RotateLeft, etc.) end up syntactically nested inside the unclosed Log2 body, which is invalid C# — you cannot declare public static members inside a method.

Root cause: Commit 7ee20d2d ("Apply suggestion from @jkotas") merged the Log2 wrapper and Log2SoftwareFallback into a single method, but the replacement added a new opening brace for the merged method without removing the existing opening brace from the old Log2SoftwareFallback body.

Fix: Remove the extra { on line 33:

publicstaticintLog2(uintvalue){// No AggressiveInlining due to large method size// Has conventional contract 0->0 (Log(0) is undefined)

✅ Correctness — value | 1 removal in Log2 is safe

The original System.Text.Encodings.Web implementation called Log2SoftwareFallback(value | 1) to guarantee the 0 → 0 contract. The merged version drops the | 1. This is safe: when value = 0, the OR-shift cascade leaves it at 0, the DeBruijn multiplication yields index 0, and Log2DeBruijn[0] = 0, so Log2(0) correctly returns 0. All other inputs are unaffected since | 1 only set the lowest bit which doesn't change floor(log2(x)) for x > 0.

✅ Correctness — Algorithm implementations match BCL software fallbacks

The PopCount(uint), PopCount(ulong), TrailingZeroCount(uint), RotateLeft, and RotateRight implementations are faithful reproductions of the well-known software fallbacks (Hamming weight, DeBruijn sequences, bit rotations). These match the deleted library-local versions exactly.

✅ Consolidation — csproj changes are correct

All three .csproj files correctly:

• Replace local file references with $(CommonPath)Polyfills\BitOperationsPolyfills.cs
• Scope the reference to '$(TargetFrameworkIdentifier)' != '.NETCoreApp'
• Use appropriate Link attributes for solution explorer organization

✅ Cleanup — BitArithmetic.cs#if NET removal is clean

Removing the #if NET/#else blocks in System.Reflection.Metadata's BitArithmetic.cs and unconditionally calling BitOperations.PopCount is correct — the polyfill provides the same implementation that was previously inlined under #else.

💡 Observation — Extra methods not currently consumed

The consolidated polyfill includes RotateRight(uint/ulong) and TrailingZeroCount(uint) which are not currently used by any of the three consuming libraries. This is fine for a shared polyfill (other libraries may need them in the future), but worth noting for awareness.

Generated by Code Review for issue #126391 · ◷

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR consolidates downlevel (TargetFrameworkIdentifier != .NETCoreApp) polyfills for System.Numerics.BitOperations and Memory-based System.IO.Stream APIs into src/libraries/Common/src/Polyfills/, and updates multiple library projects to consume the shared implementations instead of library-local copies.

Changes:

• Expanded Common’s BitOperations polyfill and switched affected libraries to reference it from Common/src/Polyfills.
• Split stream polyfills into StreamPolyfills (no System.Memory dependencies) and StreamMemoryPolyfills (Memory-based APIs) and updated consuming projects accordingly.
• Removed library-local polyfill sources (e.g., in System.IO.Hashing and System.Text.Encodings.Web) and updated call sites (e.g., System.Reflection.Metadata) to use BitOperations.PopCount.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Text.Json/src/System.Text.Json.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj	Replaces local BitOperations polyfill with shared BitOperationsPolyfills.
src/libraries/System.Text.Encodings.Web/src/Polyfills/System.Numerics.BitOperations.netstandard20.cs	Deletes library-local Log2 polyfill implementation.
src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs	Replaces BitArithmetic.CountBits usage with BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs	Switches external table count calculation to BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs	Removes CountBits helpers and uses BitOperations.PopCount for alignment asserts.
src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj	Adds shared BitOperationsPolyfills.cs for non-.NETCoreApp targets.
src/libraries/System.Net.ServerSentEvents/src/System.Net.ServerSentEvents.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj	Adds StreamPolyfills + StreamMemoryPolyfills for downlevel targets.
src/libraries/System.IO.Pipelines/src/System.IO.Pipelines.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/System.IO.Hashing/src/System/IO/Hashing/BitOperations.cs	Deletes library-local RotateLeft polyfill.
src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj	References shared BitOperationsPolyfills + StreamPolyfills for downlevel targets.
src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs	Adds Memory-based stream polyfills (ReadAsync, Write, WriteAsync) for downlevel TFMs.
src/libraries/Common/src/Polyfills/BitOperationsPolyfills.cs	Expands shared BitOperations polyfill surface (Log2/PopCount/rotates/TZC).

Comments suppressed due to low confidence (2)

src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:19

• In ReadAsync(Memory) the non-array-backed path rents a buffer from ArrayPool and then calls stream.ReadAsync(...) inline. If stream.ReadAsync throws synchronously (e.g., ObjectDisposedException / argument validation) before returning a Task, the rented buffer won't be returned and will leak. Consider capturing the Task inside a try/catch and returning the rented buffer in the catch before rethrowing, then passing the Task to FinishReadAsync for the awaited path.
  src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:76
• In WriteAsync(ReadOnlyMemory) the non-array-backed path rents a buffer and then calls stream.WriteAsync(...) without guarding against synchronous exceptions. If stream.WriteAsync throws before returning a Task, the rented buffer will leak because FinishWriteAsync won't run. Capture the Task in a try/catch (or use a try/finally around the call) and ensure ArrayPool.Return happens on the synchronous-throw path as well.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Fixed in commit d5a297c. Updated TagToTokenTests.cs to call BitOperations.PopCount(...) directly instead of the deleted BitArithmetic.CountBits(...), and added using System.Numerics;.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Already fixed in commit d5a297c — TagToTokenTests.cs now calls BitOperations.PopCount(...) directly with using System.Numerics;.

[Copilot]

Follow-up to #126287 — consolidates library-local BitOperations, Stream, StringBuilder, and Stack polyfills under src/libraries/Common/src/Polyfills/.

Changes

Common/src/Polyfills/BitOperationsPolyfills.cs

Expanded the existing polyfill (which only had RotateLeft(uint)) with the full set of methods needed by the affected libraries:

• Log2(uint) (deBruijn-based)
• PopCount(uint) and PopCount(ulong) (Hamming weight)
• RotateLeft(ulong), RotateRight(uint), RotateRight(ulong)
• TrailingZeroCount(uint) (deBruijn-based)

Common/src/Polyfills/StreamPolyfills.cs

Merged ReadExactly and CopyToAsync from the now-deleted Common/src/System/IO/StreamExtensions.netstandard.cs into the existing Stream polyfill (no System.Memory/System.Buffers dependencies).

Common/src/Polyfills/StreamMemoryPolyfills.cs(new)

New file containing the Memory-dependent stream extension methods, split out from StreamPolyfills.cs to avoid pulling in extra package references for consumers that don't need them:

• ReadAsync(Memory<byte>, CancellationToken)
• Write(ReadOnlyMemory<byte>)
• WriteAsync(ReadOnlyMemory<byte>, CancellationToken)

Common/src/Polyfills/StringBuilderPolyfills.cs(new)

Moved from Common/src/System/Text/StringBuilderExtensions.cs and reworked to follow polyfill directory conventions (file-scoped namespace, extension(StringBuilder) block syntax):

• Append(ReadOnlySpan<char>)

Common/src/Polyfills/StackPolyfills.cs(new)

Moved from System.Text.Json-local StackExtensions.netstandard.cs and reworked to follow polyfill directory conventions (file-scoped namespace, traditional extension methods matching DictionaryPolyfills.cs style for generic types):

• TryPeek<T>(out T)
• TryPop<T>(out T)

System.IO.Hashing

• Removed local System/IO/Hashing/BitOperations.cs (had RotateLeft(uint/ulong))
• Updated .csproj to reference BitOperationsPolyfills.cs and StreamPolyfills.cs

System.Text.Encodings.Web

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had Log2)
• Updated .csproj to reference BitOperationsPolyfills.cs

Microsoft.Bcl.Memory

• Removed local Polyfills/System.Numerics.BitOperations.netstandard20.cs (had TrailingZeroCount, RotateLeft(uint), RotateRight(uint))
• Updated .csproj to reference BitOperationsPolyfills.cs (which is a strict superset)

System.Reflection.Metadata

• Deleted BitArithmetic.CountBits(uint/ulong) wrapper methods; all call sites (BitArithmetic.Align, MetadataSizes, PEHeaderBuilder) now call BitOperations.PopCount directly
• Updated TagToTokenTests.cs to call BitOperations.PopCount directly (fixes .NETFramework build break)
• Added BitOperationsPolyfills.cs include in .csproj for non-.NETCoreApp targets

System.Net.ServerSentEvents, System.Text.Json, Microsoft.Extensions.Logging.Console, System.Net.Http.WinHttpHandler.Functional.Tests

• Reference StreamMemoryPolyfills.cs (use Memory-based stream methods)

System.IO.Pipelines

• References both StreamPolyfills.cs (uses CopyToAsync) and StreamMemoryPolyfills.cs (uses ReadAsync(Memory<byte>))

System.Speech, System.ServiceModel.Syndication

• Reference StreamPolyfills.cs only (use ReadExactly); no extra System.Memory/System.Buffers/System.Threading.Tasks.Extensions package references needed

System.Text.Json

• Updated .csproj to reference StringBuilderPolyfills.cs and StackPolyfills.cs from Common/src/Polyfills/
• Removed local StackExtensions.netstandard.cs

Note

This PR description was generated with the assistance of GitHub Copilot.

[github-actions]

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #126391

Holistic Assessment

Motivation: This PR continues the consolidation work from #126287 by merging three separate library-local BitOperations polyfill implementations (from System.IO.Hashing, System.Text.Encodings.Web, and System.Reflection.Metadata) into the shared Common/src/Polyfills/BitOperationsPolyfills.cs. The motivation is clear and well-justified — eliminating duplicate code and reducing #if NET/#else blocks.

Approach: The approach is consistent with the prior PR's pattern: centralize polyfills in a shared directory, update .csproj files to link the shared file for non-.NETCoreApp targets, and remove library-local copies. The consolidation is straightforward.

Summary: ❌ Needs Changes. The PR has a clear compilation error: a double opening brace in the Log2 method body that leaves the method unclosed. This must be fixed before merge. All other changes are clean.

---

Detailed Findings

❌ Compilation Error — Double { in Log2 method (BitOperationsPolyfills.cs:32-33)

Merge-blocking. The Log2 method has two consecutive opening braces but only one closing brace, creating an unbalanced brace structure that will not compile:

publicstaticintLog2(uintvalue){// ← line 32: method body opens{// ← line 33: extra block opens (from the old Log2SoftwareFallback body)// ... implementation ...returnUnsafe.AddByteOffset(...);}// ← line 50: closes only the inner block// method body is never closed!

Brace analysis of the entire file shows 8 opening braces vs 7 closing braces (final nesting depth 1, expected 0). All subsequent methods (PopCount, RotateLeft, etc.) end up syntactically nested inside the unclosed Log2 body, which is invalid C# — you cannot declare public static members inside a method.

Root cause: Commit 7ee20d2d ("Apply suggestion from @jkotas") merged the Log2 wrapper and Log2SoftwareFallback into a single method, but the replacement added a new opening brace for the merged method without removing the existing opening brace from the old Log2SoftwareFallback body.

Fix: Remove the extra { on line 33:

publicstaticintLog2(uintvalue){// No AggressiveInlining due to large method size// Has conventional contract 0->0 (Log(0) is undefined)

✅ Correctness — value | 1 removal in Log2 is safe

The original System.Text.Encodings.Web implementation called Log2SoftwareFallback(value | 1) to guarantee the 0 → 0 contract. The merged version drops the | 1. This is safe: when value = 0, the OR-shift cascade leaves it at 0, the DeBruijn multiplication yields index 0, and Log2DeBruijn[0] = 0, so Log2(0) correctly returns 0. All other inputs are unaffected since | 1 only set the lowest bit which doesn't change floor(log2(x)) for x > 0.

✅ Correctness — Algorithm implementations match BCL software fallbacks

The PopCount(uint), PopCount(ulong), TrailingZeroCount(uint), RotateLeft, and RotateRight implementations are faithful reproductions of the well-known software fallbacks (Hamming weight, DeBruijn sequences, bit rotations). These match the deleted library-local versions exactly.

✅ Consolidation — csproj changes are correct

All three .csproj files correctly:

• Replace local file references with $(CommonPath)Polyfills\BitOperationsPolyfills.cs
• Scope the reference to '$(TargetFrameworkIdentifier)' != '.NETCoreApp'
• Use appropriate Link attributes for solution explorer organization

✅ Cleanup — BitArithmetic.cs#if NET removal is clean

Removing the #if NET/#else blocks in System.Reflection.Metadata's BitArithmetic.cs and unconditionally calling BitOperations.PopCount is correct — the polyfill provides the same implementation that was previously inlined under #else.

💡 Observation — Extra methods not currently consumed

The consolidated polyfill includes RotateRight(uint/ulong) and TrailingZeroCount(uint) which are not currently used by any of the three consuming libraries. This is fine for a shared polyfill (other libraries may need them in the future), but worth noting for awareness.

Generated by Code Review for issue #126391 · ◷

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR consolidates downlevel (TargetFrameworkIdentifier != .NETCoreApp) polyfills for System.Numerics.BitOperations and Memory-based System.IO.Stream APIs into src/libraries/Common/src/Polyfills/, and updates multiple library projects to consume the shared implementations instead of library-local copies.

Changes:

• Expanded Common’s BitOperations polyfill and switched affected libraries to reference it from Common/src/Polyfills.
• Split stream polyfills into StreamPolyfills (no System.Memory dependencies) and StreamMemoryPolyfills (Memory-based APIs) and updated consuming projects accordingly.
• Removed library-local polyfill sources (e.g., in System.IO.Hashing and System.Text.Encodings.Web) and updated call sites (e.g., System.Reflection.Metadata) to use BitOperations.PopCount.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Text.Json/src/System.Text.Json.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj	Replaces local BitOperations polyfill with shared BitOperationsPolyfills.
src/libraries/System.Text.Encodings.Web/src/Polyfills/System.Numerics.BitOperations.netstandard20.cs	Deletes library-local Log2 polyfill implementation.
src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaderBuilder.cs	Replaces BitArithmetic.CountBits usage with BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs	Switches external table count calculation to BitOperations.PopCount.
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/BitArithmetic.cs	Removes CountBits helpers and uses BitOperations.PopCount for alignment asserts.
src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj	Adds shared BitOperationsPolyfills.cs for non-.NETCoreApp targets.
src/libraries/System.Net.ServerSentEvents/src/System.Net.ServerSentEvents.csproj	Switches downlevel stream polyfill include to StreamMemoryPolyfills.
src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj	Adds StreamPolyfills + StreamMemoryPolyfills for downlevel targets.
src/libraries/System.IO.Pipelines/src/System.IO.Pipelines.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/System.IO.Hashing/src/System/IO/Hashing/BitOperations.cs	Deletes library-local RotateLeft polyfill.
src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj	References shared BitOperationsPolyfills + StreamPolyfills for downlevel targets.
src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj	Replaces prior stream polyfill include with StreamPolyfills + StreamMemoryPolyfills.
src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs	Adds Memory-based stream polyfills (ReadAsync, Write, WriteAsync) for downlevel TFMs.
src/libraries/Common/src/Polyfills/BitOperationsPolyfills.cs	Expands shared BitOperations polyfill surface (Log2/PopCount/rotates/TZC).

Comments suppressed due to low confidence (2)

src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:19

• In ReadAsync(Memory) the non-array-backed path rents a buffer from ArrayPool and then calls stream.ReadAsync(...) inline. If stream.ReadAsync throws synchronously (e.g., ObjectDisposedException / argument validation) before returning a Task, the rented buffer won't be returned and will leak. Consider capturing the Task inside a try/catch and returning the rented buffer in the catch before rethrowing, then passing the Task to FinishReadAsync for the awaited path.
  src/libraries/Common/src/Polyfills/StreamMemoryPolyfills.cs:76
• In WriteAsync(ReadOnlyMemory) the non-array-backed path rents a buffer and then calls stream.WriteAsync(...) without guarding against synchronous exceptions. If stream.WriteAsync throws before returning a Task, the rented buffer will leak because FinishWriteAsync won't run. Capture the Task in a try/catch (or use a try/finally around the call) and ensure ArrayPool.Return happens on the synchronous-throw path as well.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Fixed in commit d5a297c. Updated TagToTokenTests.cs to call BitOperations.PopCount(...) directly instead of the deleted BitArithmetic.CountBits(...), and added using System.Numerics;.

[Copilot]

src\libraries\System.Reflection.Metadata\tests\Metadata\TagToTokenTests.cs(164,89): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'BitArithmetic' does not contain a definition for 'CountBits'
...

Already fixed in commit d5a297c — TagToTokenTests.cs now calls BitOperations.PopCount(...) directly with using System.Numerics;.