Skip to content
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

Add System.Math API overloads for nint/nuint #43733

Closed
Sergio0694 opened this issue Oct 22, 2020 · 9 comments · Fixed by #46647
Closed

Add System.Math API overloads for nint/nuint #43733

Sergio0694 opened this issue Oct 22, 2020 · 9 comments · Fixed by #46647
Labels
api-approved API was approved in API review, it can be implemented area-System.Numerics
Milestone

Comments

@Sergio0694
Copy link
Contributor

Sergio0694 commented Oct 22, 2020

Background and Motivation

C# 9 includes the new nint/nuint types, but it gets pretty awkward to use them in code that was previously using Math APIs such as Min or Max. This proposal is about adding overloads for the int/uint APIs in System.Math to support nint/nuint.

Right now you either have to manually write ternary operators everywhere to work around this, or just stick with int/uint.

Proposed API

namespace System
{
    public static class Math
    {
        public static nint Abs(nint value);
        public static nuint Clamp(nuint value, nuint min, nuint max);
        public static nint Clamp(nint value, nint min, nint max);
        public static nint DivRem(nint a, nint b, out nint result);
        public static nint Max(nint val1, nint val2);
        public static nuint Max(nuint val1, nuint val2);
        public static nint Min(nint val1, nint val2);
        public static nuint Min(nuint val1, nuint val2);
        public static nint Sign(nint value);
    }
}
@Sergio0694 Sergio0694 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 22, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Oct 22, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@tannergooding
Copy link
Member

Looking at the other APIs, there is also:

BigMul
Clamp
DivRem

I don't think BigMul makes sense, for the same reason it won't work for long; but Clamp and DivRem seem like reasonable overloads to provide.

@tannergooding tannergooding added area-System.Numerics and removed untriaged New issue has not been triaged by the area owner labels Oct 22, 2020
@ghost
Copy link

ghost commented Oct 22, 2020

Tagging subscribers to this area: @tannergooding, @pgovind, @jeffhandley
See info in area-owners.md if you want to be subscribed.

@tannergooding tannergooding added this to the Future milestone Oct 22, 2020
@Sergio0694
Copy link
Contributor Author

Added Clamp and DivRem, was looking at the .NET Standard reference and missed those 😅

@tannergooding tannergooding added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Oct 22, 2020
@tannergooding
Copy link
Member

Thanks!

Notably we don't expose unsigned DivRem overloads today. Maybe we should....

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Nov 17, 2020
@terrajobst
Copy link
Member

terrajobst commented Nov 17, 2020

Video

Looks good as proposed

namespace System
{
    public static class Math
    {
        public static nint Abs(nint value);
        public static nuint Clamp(nuint value, nuint min, nuint max);
        public static nint Clamp(nint value, nint min, nint max);
        public static nint DivRem(nint a, nint b, out nint result);
        public static nint Max(nint val1, nint val2);
        public static nuint Max(nuint val1, nuint val2);
        public static nint Min(nint val1, nint val2);
        public static nuint Min(nuint val1, nuint val2);
        public static nint Sign(nint value);
    }
}

@tannergooding
Copy link
Member

See the decision on #42156 for DivRem and the value tuple pattern.

@Praveen-Rai
Copy link

Can we also add Math.Pow() to the list ? Presently it operates only on doubles.

@svick
Copy link
Contributor

svick commented Dec 27, 2020

@Praveen-Rai I think that should be a separate proposal, considering the issues that were previously raised about it at #25784.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 6, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 8, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Numerics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants