Fix pattern matching to enforce consistent variable bindings (#65)

Pattern variables (e.g. x_) appearing multiple times in a pattern must
bind to the same value. Previously, bindings were merged with extend()
which silently allowed conflicting values, causing expressions like
Sqrt[x] to incorrectly match x_ when it was already bound to x.

Author: ad-si

src/evaluator/pattern_matching.rs

@@ -1,6 +1,30 @@
 #[allow(unused_imports)]
 use super::*;
 
+/// Merge new bindings into existing bindings, checking for consistency.
+/// If a variable name already has a binding, the new value must be
+/// structurally equal. Returns false if there is a conflict.
+fn merge_bindings(
+  existing: &mut Vec<(String, Expr)>,
+  new: Vec<(String, Expr)>,
+) -> bool {
+  for (name, value) in new {
+    if name.is_empty() {
+      continue;
+    }
+    if let Some((_, existing_value)) = existing.iter().find(|(n, _)| *n == name)
+    {
+      if !expr_equal(existing_value, &value) {
+        return false;
+      }
+      // Already bound to the same value, skip duplicate
+    } else {
+      existing.push((name, value));
+    }
+  }
+  true
+}
+
 /// Perform nested access on an association: assoc["a", "b"] -> assoc["a"]["b"]
 pub fn association_nested_access(
   var_name: &str,
@@ -332,8 +356,9 @@ pub fn try_one_identity_match(
   // try matching the expression against it
   if required_indices.len() == 1 {
     let req_pat = &pat_args[required_indices[0]];
-    if let Some(mut req_bindings) = match_pattern(expr, req_pat) {
-      req_bindings.extend(bindings);
+    if let Some(mut req_bindings) = match_pattern(expr, req_pat)
+      && merge_bindings(&mut req_bindings, bindings)
+    {
       return Some(req_bindings);
     }
   }
@@ -1412,8 +1437,8 @@ fn match_args_with_sequences(
     if let Some(mut bindings) = match_pattern(&expr_args[0], pat)
       && let Some(rest_bindings) =
         match_args_with_sequences(&expr_args[1..], rest_pats)
+      && merge_bindings(&mut bindings, rest_bindings)
     {
-      bindings.extend(rest_bindings);
       return Some(bindings);
     }
     None
@@ -1592,7 +1617,9 @@ pub fn match_pattern(
           let mut bindings = Vec::new();
           for (p, e) in pat_items.iter().zip(expr_items.iter()) {
             if let Some(b) = match_pattern(e, p) {
-              bindings.extend(b);
+              if !merge_bindings(&mut bindings, b) {
+                return None;
+              }
             } else {
               return None;
             }
@@ -1639,7 +1666,9 @@ pub fn match_pattern(
           let mut bindings = Vec::new();
           for (p, e) in pat_args.iter().zip(expr_args.iter()) {
             if let Some(b) = match_pattern(e, p) {
-              bindings.extend(b);
+              if !merge_bindings(&mut bindings, b) {
+                return None;
+              }
             } else {
               return None;
             }
@@ -1679,12 +1708,16 @@ pub fn match_pattern(
         }
         let mut bindings = Vec::new();
         if let Some(b) = match_pattern(expr_left, pat_left) {
-          bindings.extend(b);
+          if !merge_bindings(&mut bindings, b) {
+            return None;
+          }
         } else {
           return None;
         }
         if let Some(b) = match_pattern(expr_right, pat_right) {
-          bindings.extend(b);
+          if !merge_bindings(&mut bindings, b) {
+            return None;
+          }
         } else {
           return None;
         }

tests/interpreter_tests/syntax.rs

@@ -811,6 +811,39 @@ mod pattern_function {
       "f[a, b]^2"
     );
   }
+
+  #[test]
+  fn pattern_variable_binding_consistency() {
+    // Same named pattern variable must bind to the same value
+    // f[x_, x_] should match f[a, a] but not f[a, b]
+    assert_eq!(interpret("f[a, a] /. f[x_, x_] -> yes").unwrap(), "yes");
+    assert_eq!(interpret("f[a, b] /. f[x_, x_] -> yes").unwrap(), "f[a, b]");
+  }
+
+  #[test]
+  fn pattern_variable_no_match_sqrt_vs_symbol() {
+    // Regression test for issue #65:
+    // x_ bound to Symbol x should not match Sqrt[x]
+    assert_eq!(
+      interpret(
+        "Int[1/(x_*(a_+b_.*x_)),x_Symbol] := \
+         -Log[(a+b*x)/x]/a /; FreeQ[{a,b},x]; \
+         Int[1/(Sqrt[x]*(a + b*x)), x]"
+      )
+      .unwrap(),
+      "Int[1/(Sqrt[x]*(a + b*x)), x]"
+    );
+    // But it should still match when x_ consistently binds to x
+    assert_eq!(
+      interpret(
+        "Int[1/(x_*(a_+b_.*x_)),x_Symbol] := \
+         -Log[(a+b*x)/x]/a /; FreeQ[{a,b},x]; \
+         Int[1/(x*(a + b*x)), x]"
+      )
+      .unwrap(),
+      "-(Log[(a + b*x)/x]/a)"
+    );
+  }
 }
 
 mod none_symbol {