From dbf12b0ee6a99119c6854a663184f8df4f4c5241 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 20 Dec 2006 16:09:44 +0000 Subject: [PATCH] Fix bugs in LOOP. --- doc/flow-control.texi | 15 ++-- src/language/control/ChangeLog | 17 ++++ src/language/control/loop.c | 61 +++++++++---- tests/ChangeLog | 4 + tests/command/loop.sh | 157 ++++++++++++++++++++++++++++----- 5 files changed, 210 insertions(+), 44 deletions(-) diff --git a/doc/flow-control.texi b/doc/flow-control.texi index ea9f5a57..2c8eacd5 100644 --- a/doc/flow-control.texi +++ b/doc/flow-control.texi @@ -145,16 +145,19 @@ cause the loop to be executed only if the condition is true. If the 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}). diff --git a/src/language/control/ChangeLog b/src/language/control/ChangeLog index 2e38bb6f..b5c4224a 100644 --- a/src/language/control/ChangeLog +++ b/src/language/control/ChangeLog @@ -1,3 +1,20 @@ +Tue Dec 19 08:12:46 2006 Ben Pfaff + + Fix LOOP. Thanks to Daniel Williams + 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 * repeat.c (parse_lines): Issue an error when attempting to nest diff --git a/src/language/control/loop.c b/src/language/control/loop.c index 0657777a..dafc0ba8 100644 --- a/src/language/control/loop.c +++ b/src/language/control/loop.c @@ -86,8 +86,10 @@ static trns_proc_func loop_trns_proc, end_loop_trns_proc, break_trns_proc; 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 dataset *ds) 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 ccase *c, casenumber case_num UNUSED) 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. */ diff --git a/tests/ChangeLog b/tests/ChangeLog index 67667e5a..2e677919 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,7 @@ +Tue Dec 19 08:17:28 2006 Ben Pfaff + + * command/loop.sh: Test all the possible combinations of clauses. + Sat Dec 16 14:00:48 2006 Ben Pfaff * command/rank.sh: Fix test to allow string grouping variables. diff --git a/tests/command/loop.sh b/tests/command/loop.sh index e2b42426..2b1383fe 100755 --- a/tests/command/loop.sh +++ b/tests/command/loop.sh @@ -57,17 +57,71 @@ cd $TEMPDIR activity="create prog" cat > $TEMPDIR/loop.stat < 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 - <