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

Jump to bottom

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
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@walkerxiong
Copy link

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 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 The path to resolution is known, but the work has not been done. label Dec 21, 2021
@walkerxiong walkerxiong mentioned this issue Dec 21, 2021
Closed
@gopherbot
Copy link

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

@aclements
Copy link
Member

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.

@golang golang locked and limited conversation to collaborators Dec 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

Successfully merging a pull request may close this issue.

runtime: unsigned value do not need negative check walkerxiong/go
4 participants
@aclements @ALTree @gopherbot @walkerxiong