Skip to content

Expose nint/nuint overloads for BitOperations#58733

Merged
pgovind merged 24 commits intodotnet:mainfrom
deeprobin:issue-54390
Sep 17, 2021
Merged

Expose nint/nuint overloads for BitOperations#58733
pgovind merged 24 commits intodotnet:mainfrom
deeprobin:issue-54390

Conversation

@deeprobin
Copy link
Contributor

@deeprobin deeprobin commented Sep 6, 2021 •
edited
Loading

Description

Hello everybody, in this pull request I am implementing the proposed API to #54390 by @tannergooding.

Feel free to leave your feedback :).

Proposed API

namespace System.Numerics
{
    public static partial class BitOperations
    {
        public static bool IsPow2(nint value);
        public static bool IsPow2(nuint value);

        public static nuint RoundUpToPowerOf2(nuint value);

        public static int LeadingZeroCount(nuint value);

        public static int Log2(nuint value);

        public static int PopCount(nuint value);

        public static int TrailingZeroCount(nint value);
        public static int TrailingZeroCount(nuint value);

        public static nuint RotateLeft(nuint value, int offset);
        public static nuint RotateRight(nuint value, int offset);
    }
}

Proposal of @tannergooding

@ghost ghost added the area-System.Numerics label Sep 6, 2021
@dnfadmin
Copy link

dnfadmin commented Sep 6, 2021 •
edited
Loading

CLA assistant check
All CLA requirements met.

@deeprobin
Replace Environment.Is64BitProcess check with preprocessor variable T…
7ff6414 
…ARGET_64BIT and reimplement accidentally removed comment
@deeprobin
Copy link
Contributor Author

deeprobin commented Sep 8, 2021

Why does the Test Runner say that the methods do not exist?

@GrabYourPitchforks
Copy link
Member

GrabYourPitchforks commented Sep 9, 2021

Why does the Test Runner say that the methods do not exist?

@deeprobin Don't forget to update the ref assembly (https://raw.githubusercontent.com/dotnet/runtime/main/src/libraries/System.Runtime/ref/System.Runtime.cs). Easiest way to update it would probably be to look at how the other BitOperations API stubs are exposed, then copy + paste those as new lines, renaming methods and parameters as needed.

@danmoseley
Copy link
Member

danmoseley commented Sep 9, 2021

Better still to use the tool so there's no extra diffs:

https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/updating-ref-source.md

@deeprobin
Copy link
Contributor Author

deeprobin commented Sep 10, 2021

Better still to use the tool so there's no extra diffs:

https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/updating-ref-source.md

@danmoseley
The tool does not seem to work for me :/.

E:\dotnet\runtime>cd src

E:\dotnet\runtime\src>dotnet msbuild /t:GenerateReferenceAssemblySource
Could not execute because the application was not found or a compatible .NET SDK is not installed.
Possible reasons for this include:
  * You intended to execute a .NET program:
      The application 'msbuild' does not exist.
  * You intended to execute a .NET SDK command:
      A compatible installed .NET SDK for global.json version [6.0.100-rc.1.21430.12] from [E:\dotnet\runtime\global.json] was not found.
      Install the [6.0.100-rc.1.21430.12] .NET SDK or update [E:\dotnet\runtime\global.json] with an installed .NET SDK:
        5.0.102 [C:\PROGRA~1\dotnet\sdk]
        6.0.100-preview.7.21379.14 [C:\PROGRA~1\dotnet\sdk]

E:\dotnet\runtime\src>

@deeprobin
Copy link
Contributor Author

deeprobin commented Sep 10, 2021 •
edited
Loading

Why does the Test Runner say that the methods do not exist?

@deeprobin Don't forget to update the ref assembly (https://raw.githubusercontent.com/dotnet/runtime/main/src/libraries/System.Runtime/ref/System.Runtime.cs). Easiest way to update it would probably be to look at how the other BitOperations API stubs are exposed, then copy + paste those as new lines, renaming methods and parameters as needed.

@GrabYourPitchforks Thank you, I manually updated the ref assembly in the last commit.

@runfoapp runfoapp bot mentioned this pull request Sep 10, 2021
Closed
@runfoapp runfoapp bot mentioned this pull request Sep 13, 2021
Closed
@danmoseley
Copy link
Member

danmoseley commented Sep 16, 2021

The tool does not seem to work for me

Try using the dotnet at the root of the repo. It pulls down and uses the correct one. Alternatively it should fix it if you install the RC1 SDK

@deeprobin deeprobin marked this pull request as ready for review September 16, 2021 17:22
@tannergooding
Copy link
Member

tannergooding commented Sep 16, 2021

@pgovind could you give this a secondary review and merge if everything looks good?

pgovind
pgovind approved these changes Sep 17, 2021
View reviewed changes
Copy link

@pgovind pgovind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the work here @deeprobin !

@pgovind pgovind merged commit c00b5fa into dotnet:main Sep 17, 2021
@deeprobin
Copy link
Contributor Author

deeprobin commented Sep 18, 2021

I'm glad to hear that :).

@deeprobin deeprobin deleted the issue-54390 branch September 19, 2021 13:54
@EgorBo EgorBo mentioned this pull request Sep 23, 2021
Closed
@kunalspathak kunalspathak mentioned this pull request Sep 28, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoub stephentoub stephentoub left review comments

@jkotas jkotas jkotas left review comments

@tannergooding tannergooding tannergooding approved these changes

+1 more reviewer

@pgovind pgovind pgovind approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.Numerics

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@deeprobin @dnfadmin @GrabYourPitchforks @danmoseley @tannergooding @stephentoub @jkotas @pgovind