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

reflect: Is reflect.Type guaranteed to be hashable? #6535

Closed
bcmills opened this issue Oct 4, 2013 · 6 comments
Closed

reflect: Is reflect.Type guaranteed to be hashable? #6535

bcmills opened this issue Oct 4, 2013 · 6 comments
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Oct 4, 2013

I want to construct a cache of properties computed from a reflect.Type.
(For an example application, see http://play.golang.org/p/csLP1bBXJV)

The natural representation for that is map[reflect.Type]properties, but the correctness
of that approach depends on the implementation details of reflect.Type.

The documentation for (reflect.Type).String() says, "To test for equality, compare
the Types directly."  So that gives me some indication that what I'm trying to do
is reasonable.  However, nothing in the package tells me whether I can rely on the fact
that a reflect.Type is hashable.  (In practice, it currently is.)


The other alternatives I've considered are:

    map[struct{ pkgPath, name string }]properties - fails for unnamed types.

    map[uintptr]properties using reflect.Value.UnsafeAddr(t) - seems much worse than relying on hashability.

    map[uintptr]struct{reflect.Type, properties} using a hand-rolled hash function - very, very messy.


Since reflect.Type is hashable in practice anyway, the simplest solution would be to
document that that must be the case.

Obviously that has some drawbacks - it prevents reflect.Type from ever acquiring a
non-hashable field - but that still seems preferable to adding some kind of user-side
hashing or a new method for retrieving a unique ID.

On the other hand, if reflect.Type is not hashable, it would be good to document the
preferred workaround for constructing maps with Types as keys.
@robpike
Copy link
Contributor

robpike commented Oct 4, 2013

Comment 1:

The standard library is full of maps with reflect.Types as keys, so you can depend on
it. Perhaps it should be documented although in such matters we don't usually say what
works, only what doesn't..

Labels changed: added priority-later, documentation, removed priority-triage.

Status changed to Accepted.

@ianlancetaylor
Copy link
Member

Comment 2:

According to the language spec, the only restriction on a map key type is that the
comparison operators be fully defined.  And the documentation for the String method says
that Type values may be compared.  So it follows that Type values must be usable as map
keys.
But certainly the documentation could be more clear here.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 3:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 4:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 5:

Labels changed: added repo-main.

@bcmills bcmills added accepted Documentation Issues describing a change to documentation. labels Dec 4, 2013
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/85815 mentions this issue: reflect: explicitly state that Type values can be used as map keys

@golang golang locked and limited conversation to collaborators Jan 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

5 participants