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

math: add MaxUint, MinInt, MaxInt #28538

Closed
Silentd00m opened this issue Nov 1, 2018 · 14 comments
Closed

math: add MaxUint, MinInt, MaxInt #28538

Silentd00m opened this issue Nov 1, 2018 · 14 comments

Comments

@Silentd00m
Copy link

The math package is lacking MaxInt, MaxUint, MinInt and MinUint.

This makes writing programs that assign a max-value to int or uint without manually specifying the value impossible, if you want to run the program on both 32 and 64 bit platforms.

I have written an implementation of the constants mentioned above here: #28537

@gopherbot gopherbot added this to the Proposal milestone Nov 1, 2018
@ghost
Copy link

ghost commented Nov 2, 2018

Isn't this platform dependent, not independent?

@Silentd00m
Copy link
Author

It will give the correct max and min values for int and uint on 32 and 64 bit platforms.

I don't have access to 8 and 16 bit ones but it should work there as well due to how it's calculated.

@deanveloper
Copy link

deanveloper commented Nov 5, 2018

In your specific implementation, you have them as typed constants instead of untyped. That's probably not preferable as all other constants defined for limits are untyped constants.

implementation using unsafe: https://play.golang.org/p/VP6emWcsFSF
implementation without unsafe: https://play.golang.org/p/1thwBR0fWr3 (The name of wordSize is incorrect, as the int type does not necessarily correspond with the word size in the given environment)

@deanveloper
Copy link

deanveloper commented Nov 5, 2018

Also, while you're at it, might be a good idea to add a max uintptr as well.

Pretty sure the following is the only way to do it that results in an untyped constant:

const MinUintptr = 0
const MaxUintptr = 1<<(unsafe.Sizeof(uintptr(0))*8 - 1)<<1 - 1

Also, probably a good idea to get rid of MinUint and MinUintptr. Should just use 0. Go doesn't supply MinUint8 or MinUint16 etc.

@deanveloper
Copy link

I realized there may be a licensing issue with my implementation that doesn't use unsafe. I got the code to retrieve the number of bits that an int holds (32 << (^uint(0) >> 63)) from go101, I thought the license was open, my bad. The license is available here.

The implementation that does use unsafe and the uintptr however, I made myself and is all good to use.

cc @go101

@ianlancetaylor
Copy link
Contributor

No need to worry about licensing issues for such a short purely-functional expression.

@Silentd00m
Copy link
Author

Silentd00m commented Nov 6, 2018

I played around a bit with the changes @deanveloper proposed, especially the removal of the typing in the constants.

Having the constants typed has actually one big advantage: The constant won't overflow due to incorrect type detection/resolution.

https://play.golang.org/p/_iFMKqyauP9

When it is typed it should also protect against type narrowing.

@deanveloper
Copy link

deanveloper commented Nov 6, 2018

Having the constants typed has actually one big advantage: The constant won't overflow due to incorrect type detection/resolution.

This is correct. Pretty sure there is an issue to make the math constants typed, although I can't immediately find it. But a big advantage to having them untyped is you can do things such as the following:

dur := time.Hour * 100000000000
// can we fit dur in an int?
canFit := dur < math.MaxInt && dur > math.MinInt

or another example

func Longest(durs ...time.Duration) time.Duration {
    longestDir := math.MinInt64
    // compare each dur to longestDur and update
}

Although arguably it would be better to need to manually convert the constants to int and int64 as you need them. There's good arguments on both sides

@mvdan
Copy link
Member

mvdan commented Nov 26, 2018

I always thought these were already part of the math package. Unless there's a good reason not to include them, I think consistency is a good enough argument to add them.

@rsc
Copy link
Contributor

rsc commented Nov 28, 2018

We already have math.MaxInt32 etc for all the sized types. Adding the unsized ones seems fine. It's a little weird that they're not there. @griesemer points out that in math/bits we have a bunch of sized functions but decided to include unsized ones too. Sure, we can add Int and Uint variants.

Note that there is no math.MinUint8 etc so there should be no math.MinUint (it's always zero).

I'd want to see a compelling reason for MaxUintptr though. Let's leave that one out.

@rsc rsc modified the milestones: Proposal, Go1.13 Nov 28, 2018
@rsc rsc changed the title proposal: math: Add platform independent Max- and Min- for int and uint math: add MaxUint, MinInt, MaxInt Nov 28, 2018
@agnivade agnivade modified the milestones: Go1.13, Go1.14 May 25, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@rverma-jm
Copy link

rverma-jm commented Dec 4, 2019

can add math.AbsInt()/AvgInt() as well, even better a whole wrapper as equivalent to float type. We found usually ints are more used than floats. Appreciate a varargs implementation of min/max/avg as well :)

@griesemer
Copy link
Contributor

@rverma-jm This proposal is specifically about the functions mentioned in the title. Please do not hijack this proposal as it is now accepted. Instead open a new proposal if you want to add something else. Thanks.

@gopherbot
Copy link

Change https://golang.org/cl/247058 mentions this issue: math: add MaxUint, MinInt, MaxInt

@gopherbot
Copy link

Change https://golang.org/cl/317911 mentions this issue: doc/go1.17: Document new math constants.

gopherbot pushed a commit that referenced this issue May 20, 2021
Documents the newly introduced:
* MaxInt
* MinInt
* MaxUint

Updates #28538.
For #44513.
Fixes #46012.

Change-Id: Iab6bbcf8f76ebe105b973d5fd39b86b8cd078348
Reviewed-on: https://go-review.googlesource.com/c/go/+/317911
Trust: Heschi Kreinick <heschi@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
@rsc rsc mentioned this issue Jun 10, 2021
83 tasks
@bcmills bcmills modified the milestones: Backlog, Go1.17 Sep 13, 2021
@golang golang locked and limited conversation to collaborators Sep 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants