Add luacode.Value.IdenticalTo method

This makes equality vs. identity checks more explicit
and handles a few weird edge cases with them.
We no longer use `unique.Handle`
to build up the constant table, but it's okay
because we can use a hashing scheme
to reduce the number of comparisons per constant table insert.
This is roughly equivalent to what was happening before,
but with less runtime machinery.

Author: zombiezen

internal/luacode/code.go

@@ -343,7 +343,7 @@ func (p *parser) codeIndexed(fs *funcState, t, k expressionDescriptor) (expressi
 	isKstr := k.kind == expressionKindConstant &&
 		!k.hasJumps() &&
 		k.constantIndex() <= maxArgB &&
-		fs.constantTable[k.constantIndex()].Value().isShortString()
+		fs.Constants[k.constantIndex()].isShortString()
 	if t.kind == expressionKindUpvalue && !isKstr {
 		// [OpGetTabUp] can only index short strings.
 		// Copy the table from an upvalue to a register.

internal/luacode/funcstate.go

@@ -7,8 +7,7 @@ package luacode
 import (
 	"errors"
 	"fmt"
-	"slices"
-	"unique"
+	"hash/maphash"
 )
 
 // funcState is the mutable state associated with a [Prototype]
@@ -41,9 +40,12 @@ type funcState struct {
 	// when returning.
 	needClose bool
 
-	// constantTable will be copied into the [Prototype] Constants field,
-	// but uses [unique.Handle] to speed up lookups during build.
-	constantTable []unique.Handle[Value]
+	// constantsIndex is a mapping of [Value]
+	// to their indices in the [Prototype] Constants table.
+	// The key for a [Value] v is determined by hashValue(v).
+	constantsIndex map[uint64][]int
+	// constantsIndexSeed is used to hash the values for constantsIndex.
+	constantsIndexSeed maphash.Seed
 
 	lineInfoWriter lineInfoWriter
 }
@@ -69,11 +71,6 @@ type blockControl struct {
 //
 // Equivalent to `luaK_finish` in upstream Lua.
 func (fs *funcState) finish() error {
-	fs.Constants = make([]Value, len(fs.constantTable))
-	for i, handle := range fs.constantTable {
-		fs.Constants[i] = handle.Value()
-	}
-
 	for i, instruction := range fs.Code {
 		if i > 0 && fs.Code[i-1].IsOutTop() != instruction.IsInTop() {
 			return fmt.Errorf("internal error: instruction %d: %v follows %v",
@@ -410,16 +407,21 @@ func (fs *funcState) markToBeClosed() {
 	fs.needClose = true
 }
 
-// addConstant either inserts a constant into the function's Constants table
+// addConstant either inserts a constant into the [Prototype] Constants table
 // and returns the index of the constant
 // or returns the index of an existing identical constant in the table.
 //
 // Equivalent to `addk` in upstream Lua.
 func (fs *funcState) addConstant(k Value) int {
-	kHandle := unique.Make(k)
-	if i := slices.Index(fs.constantTable, kHandle); i >= 0 {
-		return i
+	kHash := k.hash(fs.constantsIndexSeed)
+	entries := fs.constantsIndex[kHash]
+	for _, i := range entries {
+		if k.IdenticalTo(fs.Constants[i]) {
+			return i
+		}
 	}
-	fs.constantTable = append(fs.constantTable, kHandle)
-	return len(fs.constantTable) - 1
+	fs.Constants = append(fs.Constants, k)
+	i := len(fs.Constants) - 1
+	fs.constantsIndex[kHash] = append(entries, i)
+	return i
 }

internal/luacode/parser.go

@@ -7,6 +7,7 @@ package luacode
 import (
 	"errors"
 	"fmt"
+	"hash/maphash"
 	"io"
 	"slices"
 	"strings"
@@ -175,12 +176,17 @@ func (p *parser) openFunction(prev *funcState, f *Prototype) *funcState {
 		prev:      prev,
 		Prototype: f,
 
+		constantsIndex: make(map[uint64][]int),
+
 		previousLine: f.LineDefined,
 		firstLocal:   len(p.activeVariables),
 		firstLabel:   len(p.labels),
 	}
 	if prev != nil {
 		prev.Functions = append(prev.Functions, f)
+		fs.constantsIndexSeed = prev.constantsIndexSeed
+	} else {
+		fs.constantsIndexSeed = maphash.MakeSeed()
 	}
 	p.enterBlock(fs, false)
 	return fs

internal/luacode/prototype_test.go

@@ -13,6 +13,7 @@ import (
 )
 
 var prototypeDiffOptions = cmp.Options{
+	cmp.Comparer(Value.IdenticalTo),
 	lineInfoCompareOption,
 	cmpopts.EquateEmpty(),
 }

internal/luacode/value.go

@@ -5,6 +5,7 @@
 package luacode
 
 import (
+	"hash/maphash"
 	"math"
 	"strconv"
 	"strings"
@@ -36,8 +37,8 @@ func (t valueType) noVariant() valueType {
 // Value is a subset of Lua values that can be used as constants:
 // nil, booleans, floats, integers, and strings.
 // The zero value is nil.
-// Values can be compared for equality with the == operator.
 type Value struct {
+	_    [0]func() // Prevent comparing with "==".
 	bits uint64
 	s    string
 	t    valueType
@@ -159,11 +160,11 @@ func (v Value) Unquoted() (s string, isString bool) {
 		case math.IsInf(f, -1):
 			return "-inf", false
 		default:
-		s = strconv.FormatFloat(f, 'g', -1, 64)
-		if !strings.ContainsAny(s, ".e") {
-			s += ".0"
-		}
-		return s, false
+			s = strconv.FormatFloat(f, 'g', -1, 64)
+			if !strings.ContainsAny(s, ".e") {
+				s += ".0"
+			}
+			return s, false
 		}
 	case valueTypeInteger:
 		i, _ := v.Int64(OnlyIntegral)
@@ -200,6 +201,12 @@ func (v Value) Equal(v2 Value) bool {
 	case valueTypeNil, valueTypeFalse, valueTypeTrue:
 		return v.t == v2.t
 	case valueTypeFloat:
+		if v2.IsInteger() {
+			// Float must have integer value to be equal.
+			i1, ok := v.Int64(OnlyIntegral)
+			i2, _ := v2.Int64(OnlyIntegral)
+			return ok && i1 == i2
+		}
 		f1, _ := v.Float64()
 		f2, ok := v2.Float64()
 		return ok && f1 == f2
@@ -214,6 +221,42 @@ func (v Value) Equal(v2 Value) bool {
 	}
 }
 
+// IdenticalTo reports whether two values represent the same value.
+// This is mostly the same as [Value.Equal],
+// but will report true for two NaNs, for example.
+func (v Value) IdenticalTo(v2 Value) bool {
+	if v.t != v2.t {
+		return false
+	}
+	switch v.t.noVariant() {
+	case valueTypeNil, valueTypeBoolean:
+		return true
+	case valueTypeString:
+		return v.s == v2.s
+	default:
+		return v.bits == v2.bits
+	}
+}
+
+// hash returns a hash value for v
+// such that if v1.IdenticalTo(v2),
+// then v1.hash(seed) == v2.hash(seed).
+func (v Value) hash(seed maphash.Seed) uint64 {
+	var h maphash.Hash
+	h.SetSeed(seed)
+	h.WriteByte(byte(v.t))
+	switch v.t.noVariant() {
+	case valueTypeNumber:
+		for i := range 64 / 8 {
+			h.WriteByte(byte(v.bits >> (i * 8)))
+		}
+	case valueTypeString:
+		s, _ := v.Unquoted()
+		h.WriteString(s)
+	}
+	return h.Sum64()
+}
+
 // FloatToIntegerMode is an enumeration of rounding modes for [FloatToInteger].
 type FloatToIntegerMode int
 

internal/luacode/value_test.go

@@ -34,3 +34,104 @@ func TestValueUnquoted(t *testing.T) {
 		}
 	}
 }
+
+func TestValueEqual(t *testing.T) {
+	tests := []struct {
+		v1, v2 Value
+		want   bool
+	}{
+		{Value{}, Value{}, true},
+		{BoolValue(false), Value{}, false},
+		{BoolValue(true), Value{}, false},
+		{IntegerValue(0), Value{}, false},
+		{FloatValue(0), Value{}, false},
+		{StringValue(""), Value{}, false},
+		{BoolValue(false), BoolValue(false), true},
+		{BoolValue(true), BoolValue(true), true},
+		{BoolValue(true), BoolValue(false), false},
+		{IntegerValue(42), IntegerValue(42), true},
+		{IntegerValue(42), IntegerValue(-42), false},
+		{IntegerValue(42), FloatValue(42), true},
+		{IntegerValue(math.MaxInt64 - 1023), FloatValue(math.MaxInt64 - 1023), true},
+		{IntegerValue(math.MinInt64), FloatValue(math.MinInt64), true},
+		{IntegerValue(42), FloatValue(-42), false},
+		{FloatValue(3.14), FloatValue(3.14), true},
+		{FloatValue(math.NaN()), FloatValue(42), false},
+		{FloatValue(math.NaN()), FloatValue(math.NaN()), false},
+		{StringValue(""), StringValue(""), true},
+		{StringValue(""), StringValue("123"), false},
+		{StringValue("123"), StringValue("123"), true},
+		{StringValue("123"), StringValue("456"), false},
+		{StringValue("123"), IntegerValue(123), false},
+
+		// Float values that can't be represented as an integer.
+		{IntegerValue(math.MaxInt64), FloatValue(math.MaxInt64), false},
+		{IntegerValue(math.MinInt64), FloatValue(math.MinInt64 - 1025), false},
+	}
+
+	for _, test := range tests {
+		if got := test.v1.Equal(test.v2); got && !test.want {
+			t.Errorf("%v == %v", test.v1, test.v2)
+		} else if !got && test.want {
+			t.Errorf("%v != %v", test.v1, test.v2)
+		}
+
+		if got := test.v2.Equal(test.v1); got && !test.want {
+			t.Errorf("%v == %v", test.v2, test.v1)
+		} else if !got && test.want {
+			t.Errorf("%v != %v", test.v2, test.v1)
+		}
+	}
+}
+
+func TestValueIdenticalTo(t *testing.T) {
+	identicalTests := []Value{
+		{},
+		BoolValue(false),
+		BoolValue(true),
+		IntegerValue(42),
+		IntegerValue(0),
+		IntegerValue(-42),
+		FloatValue(3.14),
+		FloatValue(42),
+		FloatValue(math.Inf(1)),
+		FloatValue(math.Inf(-1)),
+		FloatValue(math.NaN()),
+		FloatValue(math.MinInt64),
+		FloatValue(math.MaxInt64),
+		StringValue(""),
+		StringValue("abc"),
+	}
+	notIdenticalTests := []struct {
+		v1, v2 Value
+	}{
+		{BoolValue(false), Value{}},
+		{BoolValue(true), Value{}},
+		{IntegerValue(0), Value{}},
+		{FloatValue(0), Value{}},
+		{StringValue(""), Value{}},
+		{BoolValue(false), BoolValue(true)},
+		{IntegerValue(123), IntegerValue(-123)},
+		{FloatValue(123), FloatValue(-123)},
+		{FloatValue(3.14), FloatValue(-3.14)},
+		{FloatValue(math.Inf(1)), FloatValue(math.Inf(-1))},
+		{StringValue("123"), StringValue("456")},
+		{IntegerValue(123), FloatValue(123)},
+		{IntegerValue(123), StringValue("123")},
+		{FloatValue(123), StringValue("123")},
+	}
+
+	for _, v := range identicalTests {
+		if !v.IdenticalTo(v) {
+			t.Errorf("(%v).IdenticalTo(%v) = false; want true", v, v)
+		}
+	}
+	for _, test := range notIdenticalTests {
+		if test.v1.IdenticalTo(test.v2) {
+			t.Errorf("(%v).IdenticalTo(%v) = true; want false", test.v1, test.v2)
+		}
+		if test.v2.IdenticalTo(test.v1) {
+			t.Errorf("(%v).IdenticalTo(%v) = true; want false", test.v2, test.v1)
+		}
+	}
+}