Faster ReplacePD.

expreduce/ex_symbol.go

@@ -233,3 +233,19 @@ func (this *Symbol) Hash(h *hash.Hash64) {
 	(*h).Write([]byte{107, 10, 247, 23, 33, 221, 163, 156})
 	(*h).Write([]byte(this.Name))
 }
+
+func ContainsSymbol(e Ex, name string) bool {
+	asSym, isSym := e.(*Symbol)
+	if isSym {
+		return asSym.Name == name
+	}
+	asExpr, isExpr := e.(*Expression)
+	if isExpr {
+		for _, part := range asExpr.Parts {
+			if ContainsSymbol(part, name) {
+				return true
+			}
+		}
+	}
+	return false
+}

expreduce/replace.go

@@ -1,9 +1,5 @@
 package expreduce
 
-import (
-	"sort"
-)
-
 // This function assumes e and lhs have the same head and that the head is Flat.
 func FlatReplace(e *Expression, lhs *Expression, rhs Ex, orderless bool, es *EvalState) {
 	looseLhs := NewExpression([]Ex{})
@@ -44,55 +40,40 @@ func FlatReplace(e *Expression, lhs *Expression, rhs Ex, orderless bool, es *Eva
 	}
 }
 
-func ReplacePD(this Ex, es *EvalState, pm *PDManager) Ex {
-	es.Debugf("In ReplacePD(%v, pm=%v)", this, pm)
-	toReturn := this.DeepCopy()
-	// In Golang, map iterations present random order. In rare circumstances,
-	// this can lead to different return expressions for the same inputs
-	// causing inconsistency, and random issues with test cases. We want
-	// deteriministic return values from this function (and most all functions,
-	// for that matter), so we first sort the keys alphabetically.
-
-	// An expression which used to exhibit this indeterminate behavior can be
-	// found on line 68 of simplify_test.go at commit 1a7ca11. It would
-	// occasionally return 16 instead of m^2 given the input of m^2*m^2. My
-	// guess is that one of the simplify patterns has a match object named "m",
-	// but I could be wrong.
+func ReplacePDInternal(e Ex, pm *PDManager) Ex {
+	asSym, isSym := e.(*Symbol)
+	if isSym {
+		for k, def := range pm.patternDefined {
+			if k == asSym.Name {
+				// Shouldn't need the copy
+				return def
+			}
+		}
+	}
+	asExpr, isExpr := e.(*Expression)
+	if isExpr {
+		for i := range asExpr.Parts {
+			asExpr.Parts[i] = ReplacePDInternal(asExpr.Parts[i], pm)
+		}
+	}
+	return e
+}
 
-	// Isolating this issue might help me debug the issue where patterns can
-	// interfere with existing variable names. TODO: Look into this.
-	keys := []string{}
+func ReplacePD(this Ex, es *EvalState, pm *PDManager) Ex {
+	containsAny := false
 	for k := range pm.patternDefined {
-		keys = append(keys, k)
+		if ContainsSymbol(this, k) {
+			containsAny = true
+			break
+		}
 	}
-	sort.Strings(keys)
-	// First add a "UniqueDefined`" prefix to each pattern name. This will avoid
-	// Any issues where the pattern name is also a variable in one of the
-	// pattern definitions. For example, foo[k_, m_] := bar[k, m] and calling
-	// foo[m, 2] might print bar[2, 2] without this change.
-	for _, nameStr := range keys {
-		toReturn = ReplaceAll(toReturn,
-			NewExpression([]Ex{
-				&Symbol{"Rule"},
-				&Symbol{nameStr},
-				&Symbol{"UniqueDefined`" + nameStr},
-			}),
-
-			es, EmptyPD(), "")
+	if !containsAny {
+		return this
 	}
-	for _, nameStr := range keys {
-		def := pm.patternDefined[nameStr]
-		toReturn = ReplaceAll(toReturn,
-			NewExpression([]Ex{
-				&Symbol{"Rule"},
-				&Symbol{"UniqueDefined`" + nameStr},
-				def,
-			}),
 
-			es, EmptyPD(), "")
-	}
-	es.Debugf("Finished ReplacePD with toReturn=%v", toReturn)
-	return toReturn
+	// Expressions are immutable. Any time we change an expression, we must
+	// first copy it.
+	return ReplacePDInternal(this.DeepCopy(), pm)
 }
 
 // The goal of this function is to replace all matching expressions with the