search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.81k stars 17.65k forks source link

runtime,reflect: reflect.bitVector and runtime.bitvector not sync #66822

Closed qiulaidongfeng closed 6 months ago

qiulaidongfeng commented 6 months ago

Go version

tip

Output of go env in your module/workspace:

no need.

What did you do?

See https://github.com/golang/go/blob/master/src/runtime/stack.go#L580-L583 and https://github.com/golang/go/blob/master/src/reflect/type.go#L2914-L2918 .

What did you see happen?

reflect.bitVector and runtime.bitvector not sync.

What did you expect to see?

Implement matching annotations.

novitoll commented 6 months ago

runtime/stack.go has n int32. Should it be changed to n uint32 as in reflect/type.go and keeping the name struct name bitvector w/o camelCase for consistency?

If so, there are conditions where -1 is returned where we don't have a valid pcdata, and pc* functions and type pcvalueCacheEnt struct.

Please advise if it's OK to change to uint32 and use uint32(0) instead of int32(-1) for the condition above, I can send a patch.

UPDT: Change reflect.bitVector number of bits (uint32) to int32

gopherbot commented 6 months ago

Change https://go.dev/cl/578796 mentions this issue: reflect: change bitVector number of bits to int32

cherrymui commented 6 months ago

They need to match semantically, and they do. The code may not look identical at surface level. If you think the code and/or the comment can be adjusted, feel free to send a CL.

There is no need to file an issue like this. In general, this is internal implementation detail, not a user-observable issue. Thanks.