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.

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

Fix BigInteger.Rotate{Left,Right} for backport #112878

Merged
merged 4 commits into from
Feb 27, 2025

Conversation

kzrnm
Copy link
Contributor

@kzrnm kzrnm commented Feb 24, 2025

Fix #112854

In #112864, the whole algorithm is rewritten for .NET 10, but the changes in this PR are minimal.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 24, 2025
@kzrnm kzrnm force-pushed the BigIntegerFixRotateForBackport branch from 5a3dccd to 109fdbd Compare February 25, 2025 00:18
@kzrnm kzrnm force-pushed the BigIntegerFixRotateForBackport branch from 109fdbd to 72f2a64 Compare February 25, 2025 00:21
@tannergooding tannergooding merged commit 2959612 into dotnet:main Feb 27, 2025
80 of 83 checks passed
@kzrnm kzrnm deleted the BigIntegerFixRotateForBackport branch February 27, 2025 16:09
@tannergooding
Copy link
Member

/backport to release/9.0

Copy link
Contributor

Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/13572166557

@tannergooding
Copy link
Member

/backport to release/8.0

Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/13572171653

@jeffhandley
Copy link
Member

@kzrnm Can you clarify your backport requirements for this fix please? Would backporting only to .NET 9 satisfy your needs, or are you still targeting .NET 8?

@kzrnm
Copy link
Contributor Author

kzrnm commented Mar 4, 2025

@jeffhandley cc. @tannergooding
.NET 8 would also need to be backported. This bug has existed since the method was introduced in .NET 7 and should be fixed in all supported versions.

@jeffhandley
Copy link
Member

Thanks, @kzrnm. We're discussing the backports and trying to balance risk. It would help us to better understand if your scenario is blocked by .NET 8 having this issue, or if backporting only to .NET 9 would unblock you. If you are targeting .NET 9, then we might backport to .NET 9 first and give the fix some more bake time there before backporting to .NET 8.

Are you targeting .NET 8 or .NET 9 in your scenario? Thanks again.

@kzrnm
Copy link
Contributor Author

kzrnm commented Mar 4, 2025

I created the PR for backporting based on #112864 (comment), so I want to leave the decision on whether it should be backported to @tannergooding.

To be honest, I don’t have a scenario where I use this method. I reported the bug simply because it returns an obviously incorrect value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Numerics community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RotateLeft and RotateRight of BigInteger is buggy.
3 participants