Skip to content
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

The mask of shift amount in IShiftOperators<byte, int, byte> should be 7 #103900

Merged
merged 5 commits into from
Dec 3, 2024
Merged

The mask of shift amount in IShiftOperators<byte, int, byte> should be 7 #103900

merged 5 commits into from
Dec 3, 2024

Conversation

skyoxZ
Copy link
Contributor

@skyoxZ skyoxZ commented Jun 24, 2024

Fix #103506

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jun 24, 2024
@skyoxZ skyoxZ marked this pull request as draft June 24, 2024 14:19
@tannergooding
Copy link
Member

This is also notably an issue with sbyte, short, and ushort. It is also, unfortunately, a breaking change and will need a bar check.

The intent had indeed been to correctly mask, which is being done in other operations but was missed for these ones.

@skyoxZ
Copy link
Contributor Author

skyoxZ commented Jun 24, 2024

OK. I will wait for a conclusion before further action.
(I set the PR as draft just because I recognized the test should be put in ByteTests.GenericMath.cs)

This was referenced Jun 24, 2024
Open
Open
Open
Open
Closed
Closed
@jeffhandley jeffhandley mentioned this pull request Jul 20, 2024
Closed
@skyoxZ skyoxZ marked this pull request as ready for review July 20, 2024 14:44
This was referenced Jul 20, 2024
Closed
Closed
Closed
Closed
@tannergooding
Copy link
Member

@skyoxZ, just wanted to let you know that this is still on my radar.

I need to create a breaking change doc for this prior to getting it merged, which I should get to in the next couple days.

@tannergooding tannergooding added breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet labels Nov 4, 2024
@tannergooding tannergooding mentioned this pull request Nov 4, 2024
Open
@tannergooding tannergooding removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Nov 4, 2024
This was referenced Dec 3, 2024
Open
Open
@build-analysis build-analysis bot mentioned this pull request Dec 3, 2024
Closed
@tannergooding tannergooding merged commit b3ec8ee into dotnet:main Dec 3, 2024
139 checks passed
@skyoxZ skyoxZ deleted the byte-shift branch December 3, 2024 23:11
eduardo-vp pushed a commit to eduardo-vp/runtime that referenced this pull request Dec 5, 2024
@skyoxZ @tannergooding @eduardo-vp
The mask of shift amount in IShiftOperators<byte, int, byte> should…
8e344cf 
… be 7 (dotnet#103900)

* The mask of shift amount in IShiftOperators<byte, int, byte> should be 7

* Fix mask of shift amount

---------

Co-authored-by: Tanner Gooding <tagoo@outlook.com>
mikelle-rogers pushed a commit to mikelle-rogers/runtime that referenced this pull request Dec 10, 2024
@skyoxZ @tannergooding @mikelle-rogers
The mask of shift amount in IShiftOperators<byte, int, byte> should…
c5a0d43 
… be 7 (dotnet#103900)

* The mask of shift amount in IShiftOperators<byte, int, byte> should be 7

* Fix mask of shift amount

---------

Co-authored-by: Tanner Gooding <tagoo@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tannergooding tannergooding tannergooding approved these changes

Assignees
No one assigned
Labels
area-System.Numerics breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The mask of shiftAmount in IShiftOperators<byte, int, byte> should be 7 instead of 31
2 participants
@skyoxZ @tannergooding