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

runtime: unsigned value non-negative check #50287

Closed
walkerxiong opened this issue Dec 21, 2021 · 3 comments
Closed

runtime: unsigned value non-negative check #50287

walkerxiong opened this issue Dec 21, 2021 · 3 comments
Labels
NeedsFix
Milestone

Comments

@walkerxiong
Copy link

@walkerxiong walkerxiong commented Dec 21, 2021

I found that https://github.com/golang/go/blob/master/src/runtime/runtime2.go#L1089 has a non-negative check with the unsigned value, however, the compiler optimized it .

Is it necessary to delete it in the source code?

@ALTree
Copy link
Member

@ALTree ALTree commented Dec 21, 2021

From a cursory look it appears the < 0 check is indeed unnecessary. Send a patch maybe? Note that we're in the code-freeze, so it'll probably need to wait until the tree re-opens in February.

@ALTree ALTree added this to the Unplanned milestone Dec 21, 2021
@ALTree ALTree added the NeedsFix label Dec 21, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 21, 2021

Change https://golang.org/cl/373397 mentions this issue: runtime: unsigned value do not need negative check

@aclements
Copy link
Member

@aclements aclements commented Dec 21, 2021

This is not a bug, and in my opinion the existing code is more readable and more obviously correct. As you point out, the compiler will eliminate the check, so there's no cost to having it, but there is readability cost to eliminating it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants