Fix for set hashes

jfhamlin · jfhamlin · commit 2e7b90ba2726 · 2025-09-14T20:32:52.000-07:00
Signed-off-by: James Hamlin &lt;jfhamlin@gmail.com&gt;
diff --git a/pkg/compiler/analyze.go b/pkg/compiler/analyze.go
@@ -1438,7 +1438,7 @@ func (a *Analyzer) parseCaseStar(form interface{}, env Env) (*ast.Node, error) {
 		mapEntry := First(seq).(IMapEntry)
 		key := mapEntry.Key()
 		val := mapEntry.Val()
-		
+
 		// Convert key to int64
 		var keyInt int64
 		switch k := key.(type) {
@@ -1457,20 +1457,20 @@ func (a *Analyzer) parseCaseStar(form interface{}, env Env) (*ast.Node, error) {
 		default:
 			return nil, exInfo(fmt.Sprintf("case* map key must be integer, got %T", key), nil)
 		}
-		
+
 		// Extract the vector [test-constant result-expr]
 		if Count(val) != 2 {
 			return nil, exInfo("case* map value must be a 2-element vector", nil)
 		}
-		
+
 		testConstant := First(val)
 		resultExpr := second(val)
-		
+
 		// Check if this is a collision case
 		// In Clojure, entries whose keys are in skipCheck should be evaluated directly
 		// without comparison (they contain condp expressions for collision handling)
 		hasCollision := false
-		
+
 		// Check if the map key is in the skip check set
 		switch k := key.(type) {
 		case int64:
@@ -1498,7 +1498,7 @@ func (a *Analyzer) parseCaseStar(form interface{}, env Env) (*ast.Node, error) {
 				hasCollision = true
 			}
 		}
-		
+
 		// Analyze the test constant and result expression
 		var testConstantNode *ast.Node
 		if !hasCollision {
@@ -1508,13 +1508,13 @@ func (a *Analyzer) parseCaseStar(form interface{}, env Env) (*ast.Node, error) {
 				return nil, err
 			}
 		}
-		
+
 		// Analyze the result expression (or condp for collisions)
 		resultExprNode, err := a.analyzeForm(resultExpr, env)
 		if err != nil {
 			return nil, err
 		}
-		
+
 		entries = append(entries, ast.CaseEntry{
 			Key:          keyInt,
 			TestConstant: testConstantNode,
diff --git a/pkg/lang/apersistentset.go b/pkg/lang/apersistentset.go
@@ -7,6 +7,7 @@ type (
 		AFn
 		IPersistentSet
 		IHashEq
+		Hasher
 	}
 )
 
@@ -28,6 +29,21 @@ func apersistentsetEquiv(a APersistentSet, o any) bool {
 	return true
 }
 
+func apersistentsetHash(hc *uint32, a APersistentSet) uint32 {
+	if *hc != 0 {
+		return *hc
+	}
+
+	// Following Clojure's APersistentSet.hashCode logic:
+	// Sum of hash values of all elements
+	var hash uint32 = 0
+	for seq := a.Seq(); seq != nil; seq = seq.Next() {
+		hash += Hash(seq.First())
+	}
+	*hc = hash
+	return hash
+}
+
 func apersistentsetHashEq(hc *uint32, a APersistentSet) uint32 {
 	if *hc != 0 {
 		return *hc
diff --git a/pkg/lang/set.go b/pkg/lang/set.go
@@ -153,6 +153,10 @@ func (s *Set) Equiv(o any) bool {
 	return apersistentsetEquiv(s, o)
 }
 
+func (s *Set) Hash() uint32 {
+	return apersistentsetHash(&s.hash, s)
+}
+
 func (s *Set) HashEq() uint32 {
 	return apersistentsetHashEq(&s.hasheq, s)
 }
diff --git a/pkg/runtime/evalast.go b/pkg/runtime/evalast.go
@@ -470,7 +470,7 @@ func (env *environment) EvalASTCase(n *ast.Node) (interface{}, error) {
 	// Determine the lookup key based on test type
 	var lookupKey int64
 	testType := caseNode.TestType.(lang.Keyword)
-	
+
 	switch testType {
 	case lang.KWInt:
 		// For integer test type, use the value directly
@@ -493,7 +493,7 @@ func (env *environment) EvalASTCase(n *ast.Node) (interface{}, error) {
 		if caseNode.Mask != 0 {
 			lookupKey = int64(uint32(lookupKey>>uint(caseNode.Shift)) & uint32(caseNode.Mask))
 		}
-		
+
 	case lang.KWHashIdentity:
 		// Use identity hash for keywords
 		hash := lang.IdentityHash(testVal)
@@ -503,7 +503,7 @@ func (env *environment) EvalASTCase(n *ast.Node) (interface{}, error) {
 		} else {
 			lookupKey = int64(uint32(hash>>uint(caseNode.Shift)) & uint32(caseNode.Mask))
 		}
-		
+
 	case lang.KWHashEquiv:
 		// Use hash for general values
 		hash := lang.Hash(testVal)
@@ -513,7 +513,7 @@ func (env *environment) EvalASTCase(n *ast.Node) (interface{}, error) {
 		} else {
 			lookupKey = int64(uint32(hash>>uint(caseNode.Shift)) & uint32(caseNode.Mask))
 		}
-		
+
 	default:
 		return nil, fmt.Errorf("unknown test type: %v", testType)
 	}
@@ -522,7 +522,7 @@ func (env *environment) EvalASTCase(n *ast.Node) (interface{}, error) {
 	// Following Clojure's implementation: find the entry whose key matches lookupKey
 	// If that entry is marked as collision, evaluate result directly (it's a condp)
 	// Otherwise, verify the test value matches before evaluating result
-	
+
 	for _, entry := range caseNode.Entries {
 		if entry.Key != lookupKey {
 			continue
@@ -538,54 +538,54 @@ func (env *environment) EvalASTCase(n *ast.Node) (interface{}, error) {
 		} else {
 			// Non-collision case, need to verify the actual value matches
 			if testType == lang.KWInt {
-					// For integers with shift/mask, we need to verify the actual value
-					// because multiple values can map to the same key
-					if caseNode.Mask != 0 {
-						// Need to check actual value matches
-						expectedVal, err := env.EvalAST(entry.TestConstant)
-						if err != nil {
-							return nil, err
-						}
-						if lang.Equals(testVal, expectedVal) {
-							result, err := env.EvalAST(entry.ResultExpr)
-							if err != nil {
-								return nil, err
-							}
-							return result, nil
-						}
-					} else {
-						// For integers without shift/mask, the key match is sufficient
+				// For integers with shift/mask, we need to verify the actual value
+				// because multiple values can map to the same key
+				if caseNode.Mask != 0 {
+					// Need to check actual value matches
+					expectedVal, err := env.EvalAST(entry.TestConstant)
+					if err != nil {
+						return nil, err
+					}
+					if lang.Equals(testVal, expectedVal) {
 						result, err := env.EvalAST(entry.ResultExpr)
 						if err != nil {
 							return nil, err
 						}
 						return result, nil
 					}
 				} else {
-					// For hash-based dispatch, verify the actual value matches
-					expectedVal, err := env.EvalAST(entry.TestConstant)
+					// For integers without shift/mask, the key match is sufficient
+					result, err := env.EvalAST(entry.ResultExpr)
 					if err != nil {
 						return nil, err
 					}
-					
-					// Use appropriate comparison based on test type
-					var matches bool
-					if testType == lang.KWHashIdentity {
-						matches = testVal == expectedVal
-					} else {
-						matches = lang.Equals(testVal, expectedVal)
+					return result, nil
+				}
+			} else {
+				// For hash-based dispatch, verify the actual value matches
+				expectedVal, err := env.EvalAST(entry.TestConstant)
+				if err != nil {
+					return nil, err
+				}
+
+				// Use appropriate comparison based on test type
+				var matches bool
+				if testType == lang.KWHashIdentity {
+					matches = testVal == expectedVal
+				} else {
+					matches = lang.Equals(testVal, expectedVal)
+				}
+				if matches {
+					result, err := env.EvalAST(entry.ResultExpr)
+					if err != nil {
+						return nil, err
 					}
-					if matches {
-						result, err := env.EvalAST(entry.ResultExpr)
-						if err != nil {
-							return nil, err
-						}
-						return result, nil
+					return result, nil
 				}
 			}
 		}
 	}
-	
+
 	// No match found, evaluate default
 	return env.EvalAST(caseNode.Default)
 }
diff --git a/test/glojure/test_glojure/case.glj b/test/glojure/test_glojure/case.glj
@@ -2,7 +2,7 @@
 ;;
 ;; This file tests all the different paths through case evaluation:
 ;; - Compact integer dispatch
-;; - Sparse integer dispatch  
+;; - Sparse integer dispatch
 ;; - Hash-based dispatch with keywords (identity)
 ;; - Hash-based dispatch with strings and other types (equiv)
 ;; - Default expressions
@@ -21,7 +21,7 @@
     (when (and (seq? case*-form#) (= 'case* (first case*-form#)))
       (let [switch-type# (nth case*-form# 6)
             test-type# (nth case*-form# 7)]
-        (is (= ~expected-switch switch-type#) 
+        (is (= ~expected-switch switch-type#)
             (str "Expected switch type " ~expected-switch " but got " switch-type#))
         (is (= ~expected-type test-type#)
             (str "Expected test type " ~expected-type " but got " test-type#))))))
@@ -140,7 +140,7 @@
   (testing "nil as test value"
     (is (= :nil (case nil nil :nil :not-nil)))
     (is (= :not-nil (case 1 nil :nil :not-nil))))
-  
+
   (testing "nil as result"
     (is (nil? (case 1 1 nil 2 :two)))
     (is (nil? (case :foo :foo nil :bar false)))))
@@ -151,10 +151,10 @@
     (is (= :true (case true true :true false :false)))
     (is (= :false (case false true :true false :false)))
     (is (= :default (case nil true :true false :false :default))))
-  
+
   (testing "Boolean false vs nil distinction"
     ;; This is a specific test case that was failing
-    (is (= :boolean-false-result 
+    (is (= :boolean-false-result
            (case false
              false :boolean-false-result
              nil :nil-result
@@ -173,8 +173,11 @@
     (is (= :pair (case [1 2] [1 2] :pair [2 1] :reversed)))
     (is (= :map (case {:a :map :of :kws}
                      {:a :map :of :kws} :map
+                     :other)))
+    (is (= :set (case #{1 2 3}
+                     #{1 2 3} :set
                      :other))))
-  
+
   (testing "Lists must use vectors as test constants"
     ;; Lists can't be written directly as test constants because parentheses group multiple constants
     ;; But a vector can be used to match a list
@@ -197,7 +200,7 @@
         1 :one
         2 :two)
       (is (= 1 @counter) "Test expression should be evaluated exactly once")))
-  
+
   (testing "Result expressions not evaluated until matched"
     (let [side-effect (atom [])]
       (case 2

Original file line number	Diff line number	Diff line change
`@@ -153,6 +153,10 @@ func (s *Set) Equiv(o any) bool {`
`153`	`153`	`return apersistentsetEquiv(s, o)`
`154`	`154`	`}`
`155`	`155`
	`156`	`+func (s *Set) Hash() uint32 {`
	`157`	`+ return apersistentsetHash(&s.hash, s)`
	`158`	`+}`
	`159`	`+`
`156`	`160`	`func (s *Set) HashEq() uint32 {`
`157`	`161`	`return apersistentsetHashEq(&s.hasheq, s)`
`158`	`162`	`}`