hash/maphash: add a TinyGo version of the package - #5707
davecheney wants to merge 1 commit into
Conversation
35cb4fd to
c0da8c7
Compare
## What was wrong
TinyGo always sets the `purego` build tag (`compileopts/config.go:114`). Up to Go 1.26 that silently selected `maphash_purego.go`, which avoids `internal/runtime/maps`. Go 1.27 deleted that file, so `maphash.go` now imports `internal/runtime/maps` for every build, and that package needs `abi.MapType`, `abi.MapGroupSlots` and `abi.MapGroupSlotsBits`, which TinyGo's `internal/abi` cannot provide.
## How it was fixed
A TinyGo `src/hash/maphash` override, as is already done for `unique`. It keeps the Go 1.27 API and documentation, and reroutes three things:
- `rthash` to the existing `runtime.memhash`
- the seed to `runtime.rand`
- `comparableHash` to a new five line `runtime.comparablehash`, which wraps `hashmapInterfaceHash`, the hasher that TinyGo maps already use for interface keys
This change also adds the `pathsToOverride` entry, a trimmed copy of the upstream tests, and `hash/maphash` in `TEST_PACKAGES_FAST`.
## Two things worth knowing
1. This also corrects a second defect. `internal/abi.EscapeNonString` is a stub that calls `panic("intrinsic")` and has no compiler implementation, and Go 1.26+ `maphash.Comparable` calls it. `Comparable` thus panicked on TinyGo also on Go 1.26, where the package did compile. The override does not call it, because TinyGo does not move stacks.
2. A test failure showed that the TinyGo FNV hash seeds with `result *= uint32(seed)`, so a seed of `0` makes the hash `0`. `int(0)` and `struct{}{}` then collide. The two seed halves in `comparableHash` are made odd, instead of a change to the test, so the upstream test file needs no change to its logic.
## Known deviation
TinyGo gives `NaN` the same hash every time. This is deliberate, see `hashmapFloat64Hash` in `src/runtime/hashmap.go`. The upstream cases that require two `NaN` values to hash differently are thus removed, and the test header says so. To change this would change how TinyGo maps behave.
## Verified
- `tinygo run` works on the reproducer
- `tinygo test hash/maphash` passes on `amd64` and on `GOARCH=386`, which is the 32-bit `use64BitHash == false` branch
- `-target=wasm` and `-target=wasip1` compile, and `-target=microbit` links at 49 kB flash
- `hash`, `hash/adler32`, `hash/crc64` and `hash/fnv` still pass through the new merge directory
- `go test ./compileopts ./goenv` pass, and `gofmt` is clean
- the `wasm` tests did not run because `node`, `wasmtime` and `wasmer` are not installed, so `386` takes the place of the 32-bit run time path
Updates tinygo-org#5684
c0da8c7 to
4bb0020
Compare
|
Thank you @davecheney for working on this. The following is edited from an automated review.
var h1, h2 maphash.Hash
h1.SetSeed(s1) // two different seeds
h2.SetSeed(s2)
maphash.WriteComparable(&h1, struct{}{})
maphash.WriteComparable(&h2, struct{}{})
h1.WriteString("abc")
h2.WriteString("abc")
// h1.Sum64() == h2.Sum64(), although the seeds differ
|
On Go 1.27, TinyGo cannot compile any program that imports
hash/maphash, directly or through a dependency:What was wrong
TinyGo always sets the
puregobuild tag (compileopts/config.go:114). Up to Go 1.26 that silently selectedmaphash_purego.go, which avoidsinternal/runtime/maps. Go 1.27 deleted that file, somaphash.gonow importsinternal/runtime/mapsfor every build, and that package needsabi.MapType,abi.MapGroupSlotsandabi.MapGroupSlotsBits. TinyGo'sinternal/abicannot provide them, becauseabi.Typeis deliberately an empty struct: TinyGo represents types in its own way.How it was fixed
A TinyGo
src/hash/maphashoverride, as is already done foruniqueandreflect. It keeps the Go 1.27 API and documentation, and reroutes the three places that reach into runtime internals:runtime.memhash, withmaps.Use64BitHashruntime.memhash, with a localuse64BitHashruntime.randcomparableHashabi.TypeOfandabi.MapType.Hasherruntime.comparablehash, which wrapshashmapInterfaceHashhashmapInterfaceHashis the hasher that TinyGo maps already use for interface keys, so this adds no new hash code to the binary. The change also adds thepathsToOverrideentry, a trimmed copy of the upstream tests, andhash/maphashinTEST_PACKAGES_FAST.Two things worth knowing
internal/abi.EscapeNonStringis a stub that callspanic("intrinsic")and has no compiler implementation, and Go 1.26+maphash.Comparablecalls it.Comparablethus panicked on TinyGo also on Go 1.26, where the package did compile. The override does not call it, because TinyGo does not move stacks.result *= uint32(seed), so a seed of0makes the hash0, andint(0)andstruct{}{}then collide. The two seed halves incomparableHashare made odd. The upstream test file thus needs no change to its logic.Known deviation
TinyGo gives
NaNthe same hash every time. This is deliberate, seehashmapFloat64Hashinsrc/runtime/hashmap.go. The upstream cases that require twoNaNvalues to hash differently are thus removed, and the test header says so. To change this would change how TinyGo maps behave.Verified
tinygo runon the reproducertinygo test hash/maphash,amd64tinygo test hash/maphash,GOARCH=386use64BitHash == falsebranch-target=wasm,-target=wasip1-target=microbithash,hash/adler32,hash/crc64,hash/fnvgo test ./compileopts ./goenvgofmtThe
wasmtests did not run, becausenode,wasmtimeandwasmerare not installed here.386thus takes the place of the 32-bit run time path.This is item 1 of the eight gaps listed in #5684. The other seven are not touched here.