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

bytes/hash: unfortunate import path #34778

Open
rsc opened this issue Oct 8, 2019 · 8 comments

Comments

@rsc
Copy link
Contributor

commented Oct 8, 2019

The implementation landed for #28322, which is great (thanks!).
The choice of bytes/hash as the import path is probably not right.

Based on the discussion on #28322, it's not completely clear it will
stay limited to bytes for all time. Even if it did, it is unclear why it is
bytes/hash and not hash/bytes. Neither is great, since the package
name alone shadows either bytes or hash. As it is, the current
bytes/hash.Hash implements (non-bytes/)hash.Hash, which is
pretty confusing. Goimports won't know what to do for hash.Hash
anymore, and so on.

It seems like it should be either in package hash, with a clear name
(hash.Seeded? hash.Random?),
or else in a hash/something package with a clear name.

/cc @randall77 @bradfitz @alandonovan

@rsc rsc added this to the Go1.14 milestone Oct 8, 2019
@rsc rsc added the NeedsDecision label Oct 8, 2019
@randall77

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

We sort of settled on a byte stream API, so bytes/hash seemed reasonable. Maybe hash/stream?

The hashes in hash are all explicitly spec'd hashes, which this really isn't. Thus I don't think it should just be another type in hash.

@rsc

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2019

The fact that it's not spec'ed and can't escape the current program seems like the most important property, not the "stream" part. All hashes are streams. Maybe hash.Local or hash.Private or hash.Unspecified or hash.InMemory or hash.Undefined or ...
Just not hash.Fast.

@bcmills

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

(Just bikeshedding, feel free to ignore.)

  • hash.Builtin?
  • hash.Runtime?
@alandonovan

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

Not Runtime -- we don't guarantee it's the same one used by Go maps.

If Fast is a bad name because it draws users to it like magpies to a shiny thing, then Unspecified is a good name.

@OneOfOne

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

hash.Memory? imho Unspecified sounds "scary" to a point, specially if you're coming from C/C++.

@rsc

This comment has been minimized.

Copy link
Contributor Author

commented Oct 14, 2019

unspecified is such a long name to type.

hash/unspec
unspec.New
unspec.Hash
unspec.Seed

better ideas?

@rsc

This comment has been minimized.

Copy link
Contributor Author

commented Oct 14, 2019

or hash/maphash?

// Package maphash implements a fast, randomly seeded, collision-resistant hash function
// for use in implementing hash table-based maps. The exact algorithm is unspecified and
// may vary from implementation to implementation. The random seeds make it impossible to
// depend on (or attack) the details of the algorithm anyway.

@josharian

This comment has been minimized.

Copy link
Contributor

commented Oct 14, 2019

Short: hash.Mem, hash.Go (or hash/memhash, hash/gohash)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.