? loop.patch
Index: doc/flow-control.texi
===================================================================
RCS file: /cvsroot/pspp/pspp/doc/flow-control.texi,v
retrieving revision 1.4
diff -u -p -r1.4 flow-control.texi
--- doc/flow-control.texi	8 May 2006 01:14:16 -0000	1.4
+++ doc/flow-control.texi	19 Dec 2006 16:17:37 -0000
@@ -145,16 +145,19 @@ cause the loop to be executed only if th
 condition is false or missing before the loop contents are executed the
 first time, the loop contents are not executed at all.
 
-If index and condition clauses are both present on @cmd{LOOP}, the index
-clause is always evaluated first.
+If index and condition clauses are both present on @cmd{LOOP}, the
+index variable is always set before the condition is evaluated.  Thus,
+a condition that makes use of the index variable will always see the
+index value to be used in the next execution of the body.
 
 Specify a boolean expression for the condition on @cmd{END LOOP} to cause
-the loop to terminate if the condition is not true after the enclosed
+the loop to terminate if the condition is true after the enclosed
 code block is executed.  The condition is evaluated at the end of the
-loop, not at the beginning.
+loop, not at the beginning, so that the body of a loop with only a
+condition on @cmd{END LOOP} will always execute at least once.
 
-If the index clause and both condition clauses are not present, then the
-loop is executed MXLOOPS (@pxref{SET}) times.
+If neither the index clause nor either condition clause is
+present, then the loop is executed MXLOOPS (@pxref{SET}) times.
 
 @cmd{BREAK} also terminates @cmd{LOOP} execution (@pxref{BREAK}).
 
Index: src/language/control/ChangeLog
===================================================================
RCS file: /cvsroot/pspp/pspp/src/language/control/ChangeLog,v
retrieving revision 1.7
diff -u -p -r1.7 ChangeLog
--- src/language/control/ChangeLog	10 Dec 2006 04:12:40 -0000	1.7
+++ src/language/control/ChangeLog	19 Dec 2006 16:17:37 -0000
@@ -1,3 +1,20 @@
+Tue Dec 19 08:12:46 2006  Ben Pfaff  <blp@gnu.org>
+
+	Fix LOOP.  Thanks to Daniel Williams
+	<Daniel.E.Williams@state.or.us> for reporting one of the bugs
+	fixed here.
+	
+	* loop.c (cmd_loop): Keep track of whether we created the index
+	variable and delete it if parsing fails, instead of creating it
+	after parsing the IF clause.  This allows the index variable to be
+	used in the IF clause.  This incidentally fixes a segfault when no
+	index variable was used.  Also, return CMD_CASCADING_FAILURE if we
+	fail.
+	(parse_if_clause): Don't allow more than one IF clause.
+	(parse_index_clause): Don't allow more than one index clause.
+	Create the index variable if it doesn't exist.
+	(end_loop_trns_proc): Invert the sense of END LOOP's IF clause.
+
 Sat Dec  9 20:12:34 2006  Ben Pfaff  <blp@gnu.org>
 
 	* repeat.c (parse_lines): Issue an error when attempting to nest
Index: src/language/control/loop.c
===================================================================
RCS file: /cvsroot/pspp/pspp/src/language/control/loop.c,v
retrieving revision 1.13
diff -u -p -r1.13 loop.c
--- src/language/control/loop.c	15 Dec 2006 00:16:02 -0000	1.13
+++ src/language/control/loop.c	19 Dec 2006 16:17:37 -0000
@@ -86,8 +86,10 @@ static trns_proc_func loop_trns_proc, en
 static trns_free_func loop_trns_free;
 
 static struct loop_trns *create_loop_trns (struct dataset *);
-static bool parse_if_clause (struct lexer *, struct loop_trns *, struct expression **);
-static bool parse_index_clause (struct lexer *, struct loop_trns *, char index_var_name[]);
+static bool parse_if_clause (struct lexer *,
+                             struct loop_trns *, struct expression **);
+static bool parse_index_clause (struct dataset *, struct lexer *,
+                                struct loop_trns *, bool *created_index_var);
 static void close_loop (void *);
 
 /* LOOP. */
@@ -97,7 +99,7 @@ int
 cmd_loop (struct lexer *lexer, struct dataset *ds)
 {
   struct loop_trns *loop;
-  char index_var_name[LONG_NAME_LEN + 1];
+  bool created_index_var = false;
   bool ok = true;
 
   loop = create_loop_trns (ds);
@@ -106,21 +108,21 @@ cmd_loop (struct lexer *lexer, struct da
       if (lex_match_id (lexer, "IF")) 
         ok = parse_if_clause (lexer, loop, &loop->loop_condition);
       else
-        ok = parse_index_clause (lexer, loop, index_var_name);
+        ok = parse_index_clause (ds, lexer, loop, &created_index_var);
     }
 
-  /* Find index variable and create if necessary. */
-  if (ok && index_var_name[0] != '\0')
+  /* Clean up if necessary. */
+  if (!ok) 
     {
-      loop->index_var = dict_lookup_var (dataset_dict (ds), index_var_name);
-      if (loop->index_var == NULL)
-        loop->index_var = dict_create_var (dataset_dict (ds), 
-					   index_var_name, 0);
+      loop->max_pass_count = 0; 
+      if (loop->index_var != NULL && created_index_var)
+        {
+          dict_delete_var (dataset_dict (ds), loop->index_var);
+          loop->index_var = NULL;
+        }
     }
-  
-  if (!ok)
-    loop->max_pass_count = 0;
-  return ok ? CMD_SUCCESS : CMD_FAILURE;
+
+  return ok ? CMD_SUCCESS : CMD_CASCADING_FAILURE;
 }
 
 /* Parses END LOOP. */
@@ -189,22 +191,45 @@ static bool
 parse_if_clause (struct lexer *lexer, 
 		 struct loop_trns *loop, struct expression **condition) 
 {
+  if (*condition != NULL) 
+    {
+      lex_sbc_only_once ("IF");
+      return false;
+    }
+  
   *condition = expr_parse_pool (lexer, loop->pool, loop->ds, EXPR_BOOLEAN);
   return *condition != NULL;
 }
 
 /* Parses an indexing clause into LOOP.
-   Stores the index variable's name in INDEX_VAR_NAME[].
+   Stores true in *CREATED_INDEX_VAR if the index clause created
+   a new variable, false otherwise.
    Returns true if successful, false on failure. */
 static bool
-parse_index_clause (struct lexer *lexer, struct loop_trns *loop, char index_var_name[]) 
+parse_index_clause (struct dataset *ds, struct lexer *lexer,
+                    struct loop_trns *loop, bool *created_index_var) 
 {
+  if (loop->index_var != NULL) 
+    {
+      msg (SE, _("Only one index clause may be specified."));
+      return false;
+    }
+
   if (lex_token (lexer) != T_ID) 
     {
       lex_error (lexer, NULL);
       return false;
     }
-  strcpy (index_var_name, lex_tokid (lexer));
+
+  loop->index_var = dict_lookup_var (dataset_dict (ds), lex_tokid (lexer));
+  if (loop->index_var != NULL)
+    *created_index_var = false;
+  else
+    {
+      loop->index_var = dict_create_var_assert (dataset_dict (ds), 
+                                                lex_tokid (lexer), 0);
+      *created_index_var = true; 
+    }
   lex_get (lexer);
 
   if (!lex_force_match (lexer, '='))
@@ -336,7 +361,7 @@ end_loop_trns_proc (void *loop_, struct 
   struct loop_trns *loop = loop_;
 
   if (loop->end_loop_condition != NULL
-      && expr_evaluate_num (loop->end_loop_condition, c, case_num) != 1.0)
+      && expr_evaluate_num (loop->end_loop_condition, c, case_num) != 0.0)
     goto break_out;
 
   /* MXLOOPS limiter. */
Index: tests/ChangeLog
===================================================================
RCS file: /cvsroot/pspp/pspp/tests/ChangeLog,v
retrieving revision 1.73
diff -u -p -r1.73 ChangeLog
--- tests/ChangeLog	16 Dec 2006 22:01:19 -0000	1.73
+++ tests/ChangeLog	19 Dec 2006 16:17:38 -0000
@@ -1,3 +1,7 @@
+Tue Dec 19 08:17:28 2006  Ben Pfaff  <blp@gnu.org>
+
+	* command/loop.sh: Test all the possible combinations of clauses.
+
 Sat Dec 16 14:00:48 2006  Ben Pfaff  <blp@gnu.org>
 
 	* command/rank.sh: Fix test to allow string grouping variables.
Index: tests/command/loop.sh
===================================================================
RCS file: /cvsroot/pspp/pspp/tests/command/loop.sh,v
retrieving revision 1.16
diff -u -p -r1.16 loop.sh
--- tests/command/loop.sh	7 Nov 2006 13:00:00 -0000	1.16
+++ tests/command/loop.sh	19 Dec 2006 16:17:38 -0000
@@ -57,17 +57,71 @@ cd $TEMPDIR
 
 activity="create prog"
 cat > $TEMPDIR/loop.stat <<EOF
-data list /X 1 Y 2 ZOOLOGICAL 3.
+data list notable /x 1 y 2 z 3.
 begin data.
-125
-256
-397
-401
+121
+252
+393
+404
 end data.
-loop iterative_Variable=y to zoological by abs(zoological-y)/(zoological-y).
-print /x iterative_Variable.
-break.		/* Generates warning.
+
+echo 'Loop with index'.
+loop #i=x to y by z.
+print /#i.
+end loop.
+print/'--------'.
+execute.
+
+echo 'Loop with IF condition'.
+compute #j=x.
+loop if #j <= y.
+print /#j.
+compute #j = #j + z.
+end loop.
+print/'--------'.
+execute.
+
+echo 'Loop with END IF condition'.
+compute #k=x.
+loop.
+print /#k.
+compute #k = #k + z.
+end loop if #k > y.
+print/'--------'.
+execute.
+
+echo 'Loop with index and IF condition based on index'.
+loop #m=x to y by z if #m < 4.
+print /#m.
+end loop.
+print/'--------'.
+execute.
+
+echo 'Loop with index and END IF condition based on index'.
+loop #n=x to y by z.
+print /#n.
+end loop if #n >= 4.
+print/'--------'.
+execute.
+
+echo 'Loop with index and IF and END IF condition based on index'.
+loop #o=x to y by z if mod(#o,2) = 0.
+print /#o.
+end loop if #o >= 4.
+print/'--------'.
+execute.
+
+echo 'Loop with no conditions'.
+set mxloops = 2.
+compute #p = x.
+loop.
+print /#p.
+compute #p = #p + z.
+do if #p >= y.
+break.
+end if.
 end loop.
+print/'--------'.
 execute.
 EOF
 if [ $? -ne 0 ] ; then no_result ; fi
@@ -85,18 +139,81 @@ if [ $? -ne 0 ] ; then fail ; fi
 activity="compare results"
 perl -pi -e 's/^\s*$//g' $TEMPDIR/pspp.list
 diff  -b $TEMPDIR/pspp.list  - <<EOF
-1.1 DATA LIST.  Reading 1 record from INLINE.
-+----------+------+-------+------+
-| Variable |Record|Columns|Format|
-#==========#======#=======#======#
-|X         |     1|  1-  1|F1.0  |
-|Y         |     1|  2-  2|F1.0  |
-|ZOOLOGICAL|     1|  3-  3|F1.0  |
-+----------+------+-------+------+
-1     2.00 
-2     5.00 
-3     9.00 
-4      .00 
+Loop with index
+    1.00 
+    2.00 
+--------
+    2.00 
+    4.00 
+--------
+    3.00 
+    6.00 
+    9.00 
+--------
+--------
+Loop with IF condition
+    1.00 
+    2.00 
+--------
+    2.00 
+    4.00 
+--------
+    3.00 
+    6.00 
+    9.00 
+--------
+--------
+Loop with END IF condition
+    1.00 
+    2.00 
+--------
+    2.00 
+    4.00 
+--------
+    3.00 
+    6.00 
+    9.00 
+--------
+    4.00 
+--------
+Loop with index and IF condition based on index
+    1.00 
+    2.00 
+--------
+    2.00 
+--------
+    3.00 
+--------
+--------
+Loop with index and END IF condition based on index
+    1.00 
+    2.00 
+--------
+    2.00 
+    4.00 
+--------
+    3.00 
+    6.00 
+--------
+--------
+Loop with index and IF and END IF condition based on index
+--------
+    2.00 
+    4.00 
+--------
+--------
+--------
+Loop with no conditions
+    1.00 
+--------
+    2.00 
+    4.00 
+--------
+    3.00 
+    6.00 
+--------
+    4.00 
+--------
 EOF
 if [ $? -ne 0 ] ; then fail ; fi
 
