Skip to content

Fix BitArray.LeftShift to clear dangling high bits #123296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Copilot wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix BitArray.LeftShift to clear dangling high bits #123296

Copilot wants to merge 4 commits into from
+73 −0

Conversation

Copy link
Contributor

Copilot AI commented Jan 16, 2026 •
edited
Loading

Description

BitArray.LeftShift was not clearing unused bits in the most significant storage element after shifting. This caused PopCount to return incorrect values when bits were shifted out of range.

BitArray ba = new(33);
ba[^1] = true;

ba.LeftShift(1);
Assert.False(ba.HasAnySet());   // passes
Assert.Equal(0, ba.PopCount()); // failed: returned 1

Changes

  • Call ClearHighExtraBits() after shift operation completes in LeftShift
  • Add comprehensive Theory-based tests for both LeftShift and RightShift operations validating PopCount and HasAnySet with various bit patterns and shift counts
  • Test coverage includes edge cases around int32 boundaries (1, 8, 31, 32, 33, 64, 65 bits), various shift amounts, and bit positions at start, middle, and end
Original prompt

This section details on the original issue you should resolve

<issue_title>BitArray ShiftLeft and PopCount produce unexpected behavior</issue_title>
<issue_description>Consider:

[Fact]
public static void PopCount_LeftShiftLeavesDanglingBits()
{
    BitArray ba = new(33);
    ba[^1] = true;

    Assert.True(ba.HasAnySet());
    Assert.Equal(1, ba.PopCount());

    ba = ba.LeftShift(1);
    Assert.False(ba.HasAnySet());
    Assert.Equal(0, ba.PopCount()); // Fails here
}

This test will fail. Despite HasAnySet returning false, PopCount will return 1.

Likewise manually counting the bits will disagree with PopCount

[Fact]
public static void PopCount_LeftShiftLeavesDanglingBits()
{
    BitArray ba = new(33);
    ba[^1] = true;

    Assert.Equal(ManualCountBits(ba), ba.PopCount());

    ba = ba.LeftShift(1);
    Assert.False(ba.HasAnySet());
    Assert.Equal(ManualCountBits(ba), ba.PopCount());


    static int ManualCountBits(BitArray ba)
    {
        int count = 0;

        foreach (bool bit in ba)
        {
            if (bit)
            {
                count++;
            }
        }

        return count;
    }
}
```</issue_description>

<agent_instructions>Fix leftshift to ensure the extra bits are cleared if necessary</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@tannergooding</author><body>
Looks like `LeftShift` (and potentially other operations) are not clearing the "unused" upper bits of the most significant storage element in the backing array.

There is probably a balance between every operation ensuring that it only writes to the valid data (i.e. fixing LeftShift) and ensuring operations which might read "unused" bits mask them off (i.e. fixing PopCount, RightShift, etc)</body></comment_new>
</comments>

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Fix BitArray ShiftLeft and PopCount unexpected behavior Fix BitArray.LeftShift to clear dangling high bits Jan 17, 2026
Copilot AI requested a review from stephentoub January 17, 2026 00:25
@build-analysis build-analysis bot mentioned this pull request Jan 17, 2026
Open
stephentoub
stephentoub reviewed Jan 17, 2026
View reviewed changes
src/libraries/System.Collections/tests/BitArray/BitArray_OperatorsTests.cs
ba.LeftShift(1);
Assert.False(ba.HasAnySet());
Assert.Equal(0, ba.PopCount());
}
Copy link
Member

@stephentoub stephentoub Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, please make this test a [Theory] and validate various bit patterns and shift counts rather than only checking one combination. Also, please add similar tests with PopCount for other operations, such as RightShift, and any others that might suffer a similar problem if they inappropriately handle clearing.

Copy link
Contributor Author

Copilot AI Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expanded to [Theory] with MemberData covering various bit patterns and shift counts. Added tests for both LeftShift and RightShift with PopCount and HasAnySet validation. Tests cover edge cases around int32 boundaries (1, 8, 31, 32, 33, 64, 65 bits). See commit 066cb7c.

@stephentoub stephentoub marked this pull request as ready for review January 17, 2026 16:30
Copilot AI review requested due to automatic review settings January 17, 2026 16:30
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jan 17, 2026

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

Copilot AI reviewed Jan 17, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes a bug in BitArray.LeftShift where unused high-order bits in the most significant storage element were not being cleared after the shift operation, causing PopCount() to return incorrect values when bits were shifted out of range.

Changes:

  • Added a call to ClearHighExtraBits() in the LeftShift method to clear dangling high bits after shifting
  • Added regression test LeftShift_PopCountReturnsCorrectCount to verify the fix

Reviewed changes

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

File Description
src/libraries/System.Private.CoreLib/src/System/Collections/BitArray.cs Added ClearHighExtraBits() call after left shift operation to clear unused high bits
src/libraries/System.Collections/tests/BitArray/BitArray_OperatorsTests.cs Added regression test that verifies PopCount returns 0 after shifting the last bit out of range

Copilot AI requested a review from stephentoub January 17, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@stephentoub stephentoub Awaiting requested review from stephentoub

At least 1 approving review is required to merge this pull request.

Assignees

@stephentoub stephentoub

Copilot code review Copilot

Labels

area-System.Collections

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BitArray ShiftLeft and PopCount produce unexpected behavior

2 participants

@stephentoub