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

proposal: reflect: add a KindCount constant in Kind constants declaration #38831

Closed
yaxinlx opened this issue May 3, 2020 · 8 comments
Closed

Comments

@yaxinlx
Copy link

yaxinlx commented May 3, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

I'm writing an application which will stat the counts of all kinds of types in code.
Now I use a [reflect.UnsafePointer+1]int32 to store the counts.
But there are not any guarantees the UnsafePointer is the last constant in the declaration in later Go standard packages. It would be great to declare a KindCount in the end in the declaration.

@ianlancetaylor ianlancetaylor changed the title reflect: add a KindCount constant in Kind constants declaration. proposal: reflect: add a KindCount constant in Kind constants declaration May 4, 2020
@gopherbot gopherbot added this to the Proposal milestone May 4, 2020
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) May 4, 2020
@ianlancetaylor
Copy link
Contributor

I look for other similar examples in the standard library and came up with unicode.MaxCase. That's the only maximum-element name I could find but I could easily have missed one. There are other iota cases with no maximum element, such as ast.ObjKind (https://golang.org/pkg/go/ast/#ObjKind).

@rsc
Copy link
Contributor

rsc commented May 20, 2020

It might be okay to add reflect.MaxKind, but we'd have to be clear about the fact that the numeric value could change in future versions of Go. In general we've avoided doing that, because it breaks code that assumes that, say, [3]int and [MaxKind]int are the same type if MaxKind changes from 3 to 4.

@rsc rsc moved this from Incoming to Active in Proposals (old) May 20, 2020
@rsc
Copy link
Contributor

rsc commented May 27, 2020

/cc @robpike

@rsc
Copy link
Contributor

rsc commented May 27, 2020

Any other thoughts about adding reflect.MaxKind, a constant value which may change over time?

@robpike
Copy link
Contributor

robpike commented May 28, 2020

Code that depends on Kind is likely to have to change if any Kinds are added, so the changing value of MaxKind would be a minor consideration. Not sure if that's an argument for or against the constant, but speaking off the cuff, I'd say against: long-term stability of this list isn't guaranteed, and a programmer that depends on the list must pay attention. MaxKind doesn't help much.

To put it another way, if a programmer depended only on MaxKind, there could be surprises as new elements arise with intermediate values that are not covered by the code, while there is a chance that code that depends on UnsafePointer at the end of the list, with unchanging value, might continue to work by ignoring the extra values; the subset is consistent. But of course breakage is possible either way.

To reiterate, I am not convinced but leaning "no".

@rsc
Copy link
Contributor

rsc commented Jun 3, 2020

I agree that the original case, making a histogram indexed by kind, certainly would benefit from MaxKind. What else would? I suppose any kind of map[reflect.Kind]T could be replaced with [reflect.MaxKind]T, which would be more efficient. But when else do you want to make a map keyed by kind? New Kinds also don't happen very often, which cuts both ways. I'm trying to understand how often this will really come up. We have other enumerations like in go/token that haven't needed max either.

@rsc
Copy link
Contributor

rsc commented Jun 10, 2020

This does seem like it has limited utility. Based on the discussion above and the lack of new use cases in response to the last comment, this seems like a likely decline.

@rsc
Copy link
Contributor

rsc commented Jun 17, 2020

No change in consensus, so declined.

@rsc rsc closed this as completed Jun 17, 2020
@rsc rsc moved this from Likely Decline to Declined in Proposals (old) Jun 17, 2020
@golang golang locked and limited conversation to collaborators Jun 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

5 participants