From a07a61f1474f0cda456f2945bb15c8a8031f1181 Mon Sep 17 00:00:00 2001 From: Grant Nelson Date: Fri, 11 Oct 2024 12:04:38 -0600 Subject: [PATCH 1/2] Adding more functions to InstanceMap --- compiler/internal/typeparams/map.go | 153 +++++++++++---- compiler/internal/typeparams/map_test.go | 228 ++++++++++++++++++++++- 2 files changed, 334 insertions(+), 47 deletions(-) diff --git a/compiler/internal/typeparams/map.go b/compiler/internal/typeparams/map.go index aa16130e2..f8aa93bf8 100644 --- a/compiler/internal/typeparams/map.go +++ b/compiler/internal/typeparams/map.go @@ -1,8 +1,10 @@ package typeparams import ( + "fmt" "go/types" - "sync" + "sort" + "strings" "golang.org/x/tools/go/types/typeutil" ) @@ -25,40 +27,35 @@ type ( // instance equality, objects are compared by pointer equality, and type // arguments with types.Identical(). To reduce access complexity, we bucket // entries by a combined hash of type args. This type is generally inspired by -// typeutil.Map. +// golang.org/x/tools/go/types/typeutil/map.go type InstanceMap[V any] struct { - bootstrap sync.Once - data map[types.Object]mapBuckets[V] - len int - hasher typeutil.Hasher - zero V + data map[types.Object]mapBuckets[V] + len int + hasher typeutil.Hasher } -func (im *InstanceMap[V]) init() { - im.bootstrap.Do(func() { - im.data = map[types.Object]mapBuckets[V]{} - im.hasher = typeutil.MakeHasher() - }) +// findIndex returns bucket and index of the entry with the given key. +// If the given key isn't found, an empty bucket and -1 are returned. +func (im *InstanceMap[V]) findIndex(key Instance) (mapBucket[V], int) { + if im != nil && im.data != nil { + bucket := im.data[key.Object][typeHash(im.hasher, key.TArgs...)] + for i, candidate := range bucket { + if candidate != nil && typeArgsEq(candidate.key.TArgs, key.TArgs) { + return bucket, i + } + } + } + return nil, -1 } +// get returns the stored value for the provided key and +// a bool indicating whether the key was present in the map or not. func (im *InstanceMap[V]) get(key Instance) (V, bool) { - im.init() - - buckets, ok := im.data[key.Object] - if !ok { - return im.zero, false - } - bucket := buckets[typeHash(im.hasher, key.TArgs...)] - if len(bucket) == 0 { - return im.zero, false - } - - for _, candidate := range bucket { - if typeArgsEq(candidate.key.TArgs, key.TArgs) { - return candidate.value, true - } + if bucket, i := im.findIndex(key); i >= 0 { + return bucket[i].value, true } - return im.zero, false + var zero V + return zero, false } // Get returns the stored value for the provided key. If the key is missing from @@ -76,8 +73,11 @@ func (im *InstanceMap[V]) Has(key Instance) bool { // Set new value for the key in the map. Returns the previous value that was // stored in the map, or zero value if the key wasn't present before. -func (im *InstanceMap[V]) Set(key Instance, value V) (old V) { - im.init() +func (im *InstanceMap[V]) Set(key Instance, value V) V { + if im.data == nil { + im.data = map[types.Object]mapBuckets[V]{} + im.hasher = typeutil.MakeHasher() + } if _, ok := im.data[key.Object]; !ok { im.data[key.Object] = mapBuckets[V]{} @@ -85,26 +85,99 @@ func (im *InstanceMap[V]) Set(key Instance, value V) (old V) { bucketID := typeHash(im.hasher, key.TArgs...) // If there is already an identical key in the map, override the entry value. - for _, candidate := range im.data[key.Object][bucketID] { - if typeArgsEq(candidate.key.TArgs, key.TArgs) { - old = candidate.value + hole := -1 + bucket := im.data[key.Object][bucketID] + for i, candidate := range bucket { + if candidate == nil { + hole = i + } else if typeArgsEq(candidate.key.TArgs, key.TArgs) { + old := candidate.value candidate.value = value return old } } - // Otherwise append a new entry. - im.data[key.Object][bucketID] = append(im.data[key.Object][bucketID], &mapEntry[V]{ - key: key, - value: value, - }) + // If there is a hole in the bucket, reuse it. + if hole >= 0 { + im.data[key.Object][bucketID][hole] = &mapEntry[V]{ + key: key, + value: value, + } + } else { + // Otherwise append a new entry. + im.data[key.Object][bucketID] = append(bucket, &mapEntry[V]{ + key: key, + value: value, + }) + } im.len++ - return im.zero + var zero V + return zero } // Len returns the number of elements in the map. func (im *InstanceMap[V]) Len() int { - return im.len + if im != nil { + return im.len + } + return 0 +} + +// Delete removes the entry with the given key, if any. +// It returns true if the entry was found. +func (im *InstanceMap[V]) Delete(key Instance) bool { + if bucket, i := im.findIndex(key); i >= 0 { + // We can't compact the bucket as it + // would disturb iterators. + bucket[i] = nil + im.len-- + return true + } + return false +} + +// Iterate calls function f on each entry in the map in unspecified order. +// +// Return true from f to continue the iteration, or false to stop it. +// +// If f should mutate the map, Iterate provides the same guarantees as +// Go maps: if f deletes a map entry that Iterate has not yet reached, +// f will not be invoked for it, but if f inserts a map entry that +// Iterate has not yet reached, whether or not f will be invoked for +// it is unspecified. +func (im *InstanceMap[V]) Iterate(f func(key Instance, value V)) { + if im != nil && im.data != nil { + for _, mapBucket := range im.data { + for _, bucket := range mapBucket { + for _, e := range bucket { + if e != nil { + f(e.key, e.value) + } + } + } + } + } +} + +// Keys returns a new slice containing the set of map keys. +// The order is unspecified. +func (im *InstanceMap[V]) Keys() []Instance { + keys := make([]Instance, 0, im.Len()) + im.Iterate(func(key Instance, _ V) { + keys = append(keys, key) + }) + return keys +} + +// String returns a string representation of the map's entries. +// The entries are sorted by string representation of the entry. +func (im *InstanceMap[V]) String() string { + entries := make([]string, 0, im.Len()) + im.Iterate(func(key Instance, value V) { + entries = append(entries, fmt.Sprintf("%v:%v", key, value)) + }) + sort.Strings(entries) + return `{` + strings.Join(entries, `, `) + `}` } // typeHash returns a combined hash of several types. diff --git a/compiler/internal/typeparams/map_test.go b/compiler/internal/typeparams/map_test.go index 5018ab0d8..ed65587c7 100644 --- a/compiler/internal/typeparams/map_test.go +++ b/compiler/internal/typeparams/map_test.go @@ -31,18 +31,30 @@ func TestInstanceMap(t *testing.T) { } i3 := Instance{ Object: i1.Object, - TArgs: []types.Type{types.Typ[types.String]}, // Different type args. + TArgs: []types.Type{ // Different type args, same number. + types.Typ[types.Int], + types.Typ[types.Int], + }, + } + i4 := Instance{ + Object: i1.Object, + TArgs: []types.Type{ // This hash matches i3's hash. + types.Typ[types.String], + types.Typ[types.String], + }, + } + i5 := Instance{ + Object: i1.Object, + TArgs: []types.Type{}, // This hash matches i3's hash. } - - _ = i1 - _ = i1clone - _ = i3 - _ = i2 m := InstanceMap[string]{} // Check operations on a missing key. t.Run("empty", func(t *testing.T) { + if got, want := m.String(), `{}`; got != want { + t.Errorf("Got: empty map string %q. Want: map string %q.", got, want) + } if got := m.Has(i1); got { t.Errorf("Got: empty map contains %s. Want: empty map contains nothing.", i1) } @@ -58,6 +70,12 @@ func TestInstanceMap(t *testing.T) { if got := m.Len(); got != 1 { t.Errorf("Got: map length %d. Want: 1.", got) } + if got, want := m.String(), `{{type i1 int, int8}:abc}`; got != want { + t.Errorf("Got: map string %q. Want: map string %q.", got, want) + } + if got, want := m.Keys(), []Instance{i1}; !keysMatch(got, want) { + t.Errorf("Got: map keys %v. Want: [i1].", got) + } }) // Check operations on the existing key. @@ -77,9 +95,15 @@ func TestInstanceMap(t *testing.T) { if got := m.Get(i1clone); got != "def" { t.Errorf(`Got: getting set key returned %q. Want: "def"`, got) } + if got, want := m.String(), `{{type i1 int, int8}:def}`; got != want { + t.Errorf("Got: map string %q. Want: map string %q.", got, want) + } + if got, want := m.Keys(), []Instance{i1}; !keysMatch(got, want) { + t.Errorf("Got: map keys %v. Want: [i1].", got) + } }) - // Check for key collisions. + // Check for key collisions with different object pointer. t.Run("different object", func(t *testing.T) { if got := m.Has(i2); got { t.Errorf("Got: a new key %q is reported as present. Want: not present.", i2) @@ -94,6 +118,8 @@ func TestInstanceMap(t *testing.T) { t.Errorf("Got: map length %d. Want: 2.", got) } }) + + // Check for collisions with different type arguments and different hash. t.Run("different tArgs", func(t *testing.T) { if got := m.Has(i3); got { t.Errorf("Got: a new key %q is reported as present. Want: not present.", i3) @@ -108,4 +134,192 @@ func TestInstanceMap(t *testing.T) { t.Errorf("Got: map length %d. Want: 3.", got) } }) + + // Check for collisions with different type arguments, same hash, count. + t.Run("different tArgs hash", func(t *testing.T) { + if got := m.Has(i4); got { + t.Errorf("Got: a new key %q is reported as present. Want: not present.", i3) + } + if got := m.Set(i4, "789"); got != "" { + t.Errorf("Got: a new key %q overrode an old value %q. Want: zero value.", i3, got) + } + if got := m.Get(i4); got != "789" { + t.Errorf(`Got: getting set key %q returned: %q. Want: "789"`, i3, got) + } + if got := m.Len(); got != 4 { + t.Errorf("Got: map length %d. Want: 4.", got) + } + }) + + // Check for collisions with different type arguments and same hash, but different count. + t.Run("different tArgs count", func(t *testing.T) { + if got := m.Has(i5); got { + t.Errorf("Got: a new key %q is reported as present. Want: not present.", i3) + } + if got := m.Set(i5, "ghi"); got != "" { + t.Errorf("Got: a new key %q overrode an old value %q. Want: zero value.", i3, got) + } + if got := m.Get(i5); got != "ghi" { + t.Errorf(`Got: getting set key %q returned: %q. Want: "ghi"`, i3, got) + } + if got := m.Len(); got != 5 { + t.Errorf("Got: map length %d. Want: 5.", got) + } + if got, want := m.String(), `{{type i1 int, int8}:def, {type i1 int, int}:456, {type i1 string, string}:789, {type i1 }:ghi, {type i2 int, int8}:123}`; got != want { + t.Errorf("Got: map string %q. Want: map string %q.", got, want) + } + if got, want := m.Keys(), []Instance{i1, i2, i3, i4, i5}; !keysMatch(got, want) { + t.Errorf("Got: map keys %v. Want: [i1, i2, i3, i4, i5].", got) + } + }) + + // Check an existing entry can be deleted. + t.Run("delete existing", func(t *testing.T) { + if got := m.Delete(i3); !got { + t.Errorf("Got: deleting existing key %q returned not deleted. Want: found and deleted.", i3) + } + if got := m.Len(); got != 4 { + t.Errorf("Got: map length %d. Want: 4.", got) + } + if got := m.Has(i3); got { + t.Errorf("Got: a deleted key %q is reported as present. Want: not present.", i3) + } + if got, want := m.Keys(), []Instance{i1, i2, i4, i5}; !keysMatch(got, want) { + t.Errorf("Got: map keys %v. Want: [i1, i2, i4, i5].", got) + } + }) + + // Check deleting an existing entry has no effect. + t.Run("delete already deleted", func(t *testing.T) { + if got := m.Delete(i3); got { + t.Errorf("Got: deleting not present key %q returned as deleted. Want: not found.", i3) + } + if got := m.Len(); got != 4 { + t.Errorf("Got: map length %d. Want: 4.", got) + } + if got, want := m.Keys(), []Instance{i1, i2, i4, i5}; !keysMatch(got, want) { + t.Errorf("Got: map keys %v. Want: [i1, i2, i4, i5].", got) + } + }) + + // Check adding back a deleted value works (should fill hole in bucket). + t.Run("set deleted key", func(t *testing.T) { + if got := m.Set(i3, "jkl"); got != "" { + t.Errorf("Got: a new key %q overrode an old value %q. Want: zero value.", i3, got) + } + if got := m.Len(); got != 5 { + t.Errorf("Got: map length %d. Want: 5.", got) + } + if got, want := m.Keys(), []Instance{i1, i2, i3, i4, i5}; !keysMatch(got, want) { + t.Errorf("Got: map keys %v. Want: [i1, i2, i3, i4, i5].", got) + } + }) + + // Check deleting while iterating over the map. + t.Run("deleting while iterating", func(t *testing.T) { + notSeen := []Instance{i1, i2, i3, i4, i5} + seen := []Instance{} + kept := []Instance{} + var skipped Instance + m.Iterate(func(key Instance, value string) { + // update seen and not seen + seen = append(seen, key) + i := keyAt(notSeen, key) + if i < 0 { + t.Fatalf(`Got: failed to find current key %q in not seen. Want: it to be not seen yet.`, key) + } + notSeen = append(notSeen[:i], notSeen[i+1:]...) + + if len(seen) == 3 { + // delete the first seen key, the current key, and an unseen key + if got := m.Delete(seen[0]); !got { + t.Errorf("Got: deleting seen key %q returned not deleted. Want: found and deleted.", seen[0]) + } + if got := m.Delete(key); !got { + t.Errorf("Got: deleting current key %q returned not deleted. Want: found and deleted.", key) + } + skipped = notSeen[0] // skipped has not yet been seen so it should not be iterated over + if got := m.Delete(skipped); !got { + t.Errorf("Got: deleting not seen key %q returned not deleted. Want: found and deleted.", skipped) + } + kept = append(kept, seen[1], notSeen[1]) + } + }) + + if got := len(seen); got != 4 { + t.Errorf("Got: seen %d keys. Want: 4.", got) + } + if got := len(notSeen); got != 1 { + t.Errorf("Got: seen %d keys. Want: 1.", got) + } + if got := keyAt(notSeen, skipped); got != 0 { + t.Errorf("Got: a deleted unseen key %q was not the skipped key %q. Want: it to be skipped.", notSeen[0], skipped) + } + if got := m.Len(); got != 2 { + t.Errorf("Got: map length %d. Want: 2.", got) + } + if got := m.Keys(); !keysMatch(got, kept) { + t.Errorf("Got: map keys %v did not match kept keys. Want: %v.", got, kept) + } + }) +} + +func TestNilInstanceMap(t *testing.T) { + i1 := Instance{ + Object: types.NewTypeName(token.NoPos, nil, "i1", nil), + TArgs: []types.Type{ + types.Typ[types.Int], + types.Typ[types.Int8], + }, + } + + var m *InstanceMap[string] + if got, want := m.String(), `{}`; got != want { + t.Errorf("Got: nil map string %q. Want: map string %q.", got, want) + } + if got := m.Has(i1); got { + t.Errorf("Got: nil map contains %s. Want: nil map contains nothing.", i1) + } + if got := m.Get(i1); got != "" { + t.Errorf("Got: missing key returned %q. Want: zero value.", got) + } + if got := m.Len(); got != 0 { + t.Errorf("Got: nil map length %d. Want: 0.", got) + } + if got := m.Keys(); len(got) > 0 { + t.Errorf("Got: map keys %v did not match kept keys. Want: [].", got) + } + + // The only thing that a nil map can't safely handle is setting a key. + func() { + defer func() { + recover() + }() + m.Set(i1, "abc") + t.Errorf("Got: setting a new key on nil map did not panic, %s. Want: panic.", m.String()) + }() +} + +func keysMatch(a, b []Instance) bool { + if len(a) != len(b) { + return false + } + found := make([]bool, len(b)) + for _, v := range a { + i := keyAt(b, v) + if i < 0 || found[i] { + return false + } + found[i] = true + } + return true +} + +func keyAt(keys []Instance, target Instance) int { + for i, v := range keys { + if v.Object == target.Object && typeArgsEq(v.TArgs, target.TArgs) { + return i + } + } + return -1 } From 91464467b7099fd5da9b5a67d3066b5d23aca0d5 Mon Sep 17 00:00:00 2001 From: Grant Nelson Date: Mon, 14 Oct 2024 09:45:53 -0600 Subject: [PATCH 2/2] fixing godoc link --- compiler/internal/typeparams/map.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/internal/typeparams/map.go b/compiler/internal/typeparams/map.go index f8aa93bf8..4f6645421 100644 --- a/compiler/internal/typeparams/map.go +++ b/compiler/internal/typeparams/map.go @@ -27,7 +27,7 @@ type ( // instance equality, objects are compared by pointer equality, and type // arguments with types.Identical(). To reduce access complexity, we bucket // entries by a combined hash of type args. This type is generally inspired by -// golang.org/x/tools/go/types/typeutil/map.go +// [golang.org/x/tools/go/types/typeutil#Map] type InstanceMap[V any] struct { data map[types.Object]mapBuckets[V] len int