Exposing generic math support for unsigned right shift #68096
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
|
Tagging subscribers to this area: @dotnet/area-system-numerics Issue Detailsnull
|
| // /// <inheritdoc cref="IShiftOperators{TSelf, TResult}.op_UnsignedRightShift(TSelf, int)" /> | ||
| // static short IShiftOperators<short, short>.operator >>>(short value, int shiftAmount) => (short)((ushort)value >> shiftAmount); | ||
| /// <inheritdoc cref="IShiftOperators{TSelf, TResult}.op_UnsignedRightShift(TSelf, int)" /> | ||
| static short IShiftOperators<short, short>.operator >>>(short value, int shiftAmount) => (short)((ushort)value >>> shiftAmount); |
There was a problem hiding this comment.
Why is the cast to ushort still needed here?
There was a problem hiding this comment.
C# doesn't define most operators for byte, sbyte, short, ushort, or char (outside the op assignment ones, like += or --).
So in C#, short >>> int is actually (int)short >>> int which means -1 >>> 1 goes from 0xFFFF to 0xFFFF_FFFF and then right shifts one giving 0x7FFF_FFFF. Casting back to short then results in 0xFFFF again.
Generic math returns and operates on the same type (like F# does) and so we need to explicitly treat it as a zero-extension so we get back 0x7FFF
| /// <param name="shiftAmount">The amount by which <paramref name="value" /> is shifted right.</param> | ||
| /// <returns>The result of shifting <paramref name="value" /> right by <paramref name="shiftAmount" />.</returns> | ||
| /// <remarks>This operation is meant to perform n unsigned (otherwise known as a logical) right shift on all types.</remarks> | ||
| static abstract TResult operator >>>(TSelf value, int shiftAmount); // TODO_GENERIC_MATH: shiftAmount should be TOther |
There was a problem hiding this comment.
I assume there's an issue tracking this?
There was a problem hiding this comment.
The general design doc is tracking this at the moment. I'm just waiting for Aleksey to finish his PR and have the relevant changes stashed in a local branch
dakersnar
left a comment
dakersnar
left a comment
There was a problem hiding this comment.
Looks good to me, besides the TODO Stephen mentioned.
|
@stephentoub let me know if you have any other feedback. Given you only had two minor questions, I've merged due to the deadline to get this in. Happy to address anything else in a follow up PR. |
* Updating Microsoft.Net.Compilers.Toolset to v4.3.0-1.22215.4 for unsigned-right-shift * Exposing operator >>> (Unsigned Right Shift) * Ensure that SByte and Int16 have unsigned-right-shift operate on themselves * Adding tests covering Unsigned Right Shift
ghost
locked as resolved and limited conversation to collaborators
3 participants
No description provided.