* match.c (match_line, match_files): Another correction to how bindings

are handled in collect/coll. New bindings from the main clause and
last clause must override old bindings. This is done by some
additional set difference operations based on symbol identity.
Otherwise it is possible to end up with multiple bindings for the
same symbol, which is untidy. If the collect clause scrubs a variable
with forget and re-binds it, then combining that environment
with the previous bindings will create a duplicate.
Also, fixed a serious bug with the bindings from the last clause;
the append was wrongly put into the loop that processes the collected
lists.

2 files changed, 26 insertions, 2 deletions

diff --git a/ChangeLog b/ChangeLog
index 69f49eb2..f15584bf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
 2011-10-04  Kaz Kylheku  <kaz@kylheku.com>
 
+	* match.c (match_line, match_files): Another correction to how bindings
+	are handled in collect/coll. New bindings from the main clause and
+	last clause must override old bindings. This is done by some
+	additional set difference operations based on symbol identity.
+	Otherwise it is possible to end up with multiple bindings for the
+	same symbol, which is untidy. If the collect clause scrubs a variable
+	with forget and re-binds it, then combining that environment 
+	with the previous bindings will create a duplicate.
+	Also, fixed a serious bug with the bindings from the last clause;
+	the append was wrongly put into the loop that processes the collected
+	lists.
+
+2011-10-04  Kaz Kylheku  <kaz@kylheku.com>
+
 	* lib.c (acons): New function.
 	(set_diff): Optimize common case: list1 and list2
 	are the same, or list2 is substructure of list1.
diff --git a/match.c b/match.c
index a5dbaacd..5bb56a04 100644
--- a/match.c
+++ b/match.c
@@ -571,7 +571,12 @@ next_coll:
           for (iter = bindings_coll; iter; iter = cdr(iter)) {
             val pair = car(iter);
             val rev = cons(car(pair), nreverse(cdr(pair)));
-            bindings = nappend2(last_bindings, cons(rev, bindings));
+            bindings = cons(rev, bindings);
+          }
+
+          if (last_bindings) {
+            bindings = set_diff(bindings, last_bindings, eq_f, car_f);
+            bindings = nappend2(last_bindings, bindings);
           }
         } else if (directive == all_s || directive == some_s ||
                    directive == none_s || directive == maybe_s ||
@@ -1622,13 +1627,18 @@ repeat_spec_same_data:
         if (!bindings_coll)
           debuglf(spec_linenum, lit("nothing was collected"), nao);
 
+        bindings = set_diff(bindings, bindings_coll, eq_f, car_f);
+
         for (iter = bindings_coll; iter; iter = cdr(iter)) {
           val pair = car(iter);
           val rev = cons(car(pair), nreverse(cdr(pair)));
           bindings = cons(rev, bindings);
         }
 
-        bindings = nappend2(last_bindings, bindings);
+        if (last_bindings) {
+          bindings = set_diff(bindings, last_bindings, eq_f, car_f);
+          bindings = nappend2(last_bindings, bindings);
+        }
 
         if ((spec = rest(spec)) == nil)
           break;