Skip to content
New issue

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

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

Already on GitHub? # to your account

[Breaking Change] Generic math shifts should use the appropriate mask #43303

Closed
tannergooding opened this issue Nov 4, 2024 · 1 comment · Fixed by #44625
Closed

[Breaking Change] Generic math shifts should use the appropriate mask #43303

tannergooding opened this issue Nov 4, 2024 · 1 comment · Fixed by #44625
Assignees
Labels
breaking-change Indicates a .NET Core breaking change 🏁 Release: .NET 10 Work items for the .NET 10 release in-pr This issue will be closed (fixed) by an active pull request. 📌 seQUESTered Identifies that an issue has been imported into Quest.

Comments

@tannergooding
Copy link
Member

tannergooding commented Nov 4, 2024

Description

Introduced with dotnet/runtime#103900

Version

.NET 10 Preview 1

Previous Behavior

The behavior when utilizing generic math to perform a shift on a T could differ based on the type. In some case it would appropriately mask the shift amount by sizeof(T) - 1 and in other cases it would do no masking. This meant that "overshifting" (such as shifting a byte by 8) could result in different answers being produced than expected.

New Behavior

The implementations were updated to mask the shift amount, as appropriate, to ensure consistent behavior across all built-in integer types and with the behavior documented by the IShiftOperators interface.

Reason for change

The behavior differed from the designed behavior due to a difference in how masking works for small integer types in C#.

Feature Area

Numerics

Affected APIs

operator <<, operator >>, and operator >>> for byte, char, sbyte, short, and ushort when used via Generic Math, which requires a T constrained to where T : IShiftOperators<T, int, T> or a similar interface.


Associated WorkItem - 360644

@dotnetrepoman dotnetrepoman bot added the ⌚ Not Triaged Not triaged label Nov 4, 2024
@tannergooding tannergooding added breaking-change Indicates a .NET Core breaking change and removed ⌚ Not Triaged Not triaged Pri3 labels Nov 4, 2024
@dotnetrepoman dotnetrepoman bot added the ⌚ Not Triaged Not triaged label Nov 4, 2024
@skyoxZ
Copy link

skyoxZ commented Nov 8, 2024

A small correct: The mask was 31 (not no mask) because they were converted to int.

@gewarren gewarren self-assigned this Nov 12, 2024
@CamSoper CamSoper removed the ⌚ Not Triaged Not triaged label Jan 7, 2025
@dotnetrepoman dotnetrepoman bot added the 🗺️ mapQUEST Only used as a way to mark an issue as updated for quest. RepoMan should instantly remove it. label Jan 7, 2025
@CamSoper CamSoper added 🗺️ reQUEST Triggers an issue to be imported into Quest. and removed 🗺️ mapQUEST Only used as a way to mark an issue as updated for quest. RepoMan should instantly remove it. labels Jan 7, 2025
@sequestor sequestor bot added 📌 seQUESTered Identifies that an issue has been imported into Quest. and removed 🗺️ reQUEST Triggers an issue to be imported into Quest. labels Jan 8, 2025
@gewarren gewarren added the 🏁 Release: .NET 10 Work items for the .NET 10 release label Jan 14, 2025
@BillWagner BillWagner removed the Pri3 label Jan 22, 2025
@CamSoper CamSoper moved this from 🔖 Ready to 👀 In review in dotnet/docs January 2025 sprint project Jan 30, 2025
@dotnet-policy-service dotnet-policy-service bot added the in-pr This issue will be closed (fixed) by an active pull request. label Jan 31, 2025
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in dotnet/docs January 2025 sprint project Jan 31, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
breaking-change Indicates a .NET Core breaking change 🏁 Release: .NET 10 Work items for the .NET 10 release in-pr This issue will be closed (fixed) by an active pull request. 📌 seQUESTered Identifies that an issue has been imported into Quest.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants