dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.05k stars 4.69k forks source link

The mask of `shiftAmount` in `IShiftOperators<byte, int, byte>` should be 7 instead of 31 #103506

Open skyoxZ opened 3 months ago

skyoxZ commented 3 months ago

Description

https://github.com/dotnet/runtime/blob/0f0981f7234bcb8dc796f90fd76bd3d7a2bf2393/src/libraries/System.Private.CoreLib/src/System/Byte.cs#L1117-L1124

Reproduction Steps

using System;
using System.Numerics;

BinaryIntegerWrapper<byte> a = new(2);
Console.WriteLine((a << 11).Value);

public struct BinaryIntegerWrapper<T> : IShiftOperators<BinaryIntegerWrapper<T>, int, BinaryIntegerWrapper<T>>
            where T : IBinaryInteger<T>
{
    public T Value;

    public BinaryIntegerWrapper(T value) => Value = value;

    public static implicit operator BinaryIntegerWrapper<T>(T value) => new BinaryIntegerWrapper<T>(value);

    public static BinaryIntegerWrapper<T> operator <<(BinaryIntegerWrapper<T> value, int shiftAmount) => value.Value << shiftAmount;
    public static BinaryIntegerWrapper<T> operator >>(BinaryIntegerWrapper<T> value, int shiftAmount) => value.Value >> shiftAmount;
    public static BinaryIntegerWrapper<T> operator >>>(BinaryIntegerWrapper<T> value, int shiftAmount) => value.Value >>> shiftAmount;
}

Expected behavior

16

Actual behavior

0

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

Per this comment from @tannergooding:

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.

dotnet-policy-service[bot] commented 3 months ago

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

dotnet-policy-service[bot] commented 2 months ago

Added needs-breaking-change-doc-created label because this issue has the breaking-change label.

  1. [ ] Create and link to this issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.

Tagging @dotnet/compat for awareness of the breaking change.

jeffhandley commented 2 months ago

I've added this to the 10.0.0 milestone as it would be beneficial to apply the breaking change fix early in an LTS release.