-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Adding the BigMul API to nint, nuint, Int128, and UInt128 #117740
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
Conversation
There was a problem hiding this 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 PR adds the BigMul
API across nint
, nuint
, Int128
, and UInt128
, updates the reference assembly, and includes corresponding unit tests.
- Implemented public
BigMul
methods inSystem.Private.CoreLib
forUIntPtr
,IntPtr
,UInt128
, andInt128
- Updated
System.Runtime.ref
to declare the newBigMul
APIs - Added unit tests covering 32-bit, 64-bit, and 128-bit
BigMul
scenarios
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/libraries/System.Private.CoreLib/src/System/UIntPtr.cs | Implemented BigMul for nuint with conditional 32/64-bit logic |
src/libraries/System.Private.CoreLib/src/System/IntPtr.cs | Implemented BigMul for nint with conditional 32/64-bit logic |
src/libraries/System.Private.CoreLib/src/System/UInt128.cs | Made BigMul public and added XML docs |
src/libraries/System.Private.CoreLib/src/System/Int128.cs | Made BigMul public and added XML docs |
src/libraries/System.Runtime/ref/System.Runtime.cs | Declared BigMul on Int128 , nint , UInt128 , and nuint |
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/UIntPtrTests.cs | Added conditional tests for 32/64-bit BigMul on UIntPtr |
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/UInt128Tests.cs | Added BigMul test data and theory for UInt128 |
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/IntPtrTests.cs | Added conditional tests for 32/64-bit BigMul on IntPtr |
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Int128Tests.cs | Added BigMul test data and theory for Int128 |
Comments suppressed due to low confidence (4)
src/libraries/System.Private.CoreLib/src/System/IntPtr.cs:182
- The summary says 'unsigned' but this overload is for signed (
nint
) values; update the description to reflect signed native integers.
/// <summary>Produces the full product of two unsigned native integers.</summary>
src/libraries/System.Runtime/ref/System.Runtime.cs:4193
- The
nint.BigMul
API should be marked[CLSCompliant(false)]
in the reference assembly, matching its implementation.
public static nint BigMul(nint left, nint right, out nint lower) { throw null; }
src/libraries/System.Private.CoreLib/src/System/UIntPtr.cs:192
- C# does not support the '>>>' operator; use the logical right-shift operator '>>' since 'ulong' shifts logically.
return (uint)(result >>> 32);
src/libraries/System.Private.CoreLib/src/System/IntPtr.cs:197
- The '>>>' operator is not valid in C#; replace with '>>' to extract the high 32 bits.
return (int)(result >>> 32);
Tagging subscribers to this area: @dotnet/area-system-numerics |
/ba-g unrelated WASM timeouts |
This resolves #114731