From f33178b801e1d8e932c2ef411d3985017627c512 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 1 Dec 2006 06:51:08 +0000 Subject: [PATCH] * compute.c: Fix bug #17422, which reports that a variable created by assignment in a COMPUTE command could not be used in the computation expression. --- src/language/xforms/ChangeLog | 16 +++++++ src/language/xforms/compute.c | 76 +++++++++++++++++--------------- tests/ChangeLog | 6 +++ tests/automake.mk | 1 + tests/bugs/compute-sum.sh | 83 +++++++++++++++++++++++++++++++++++ 5 files changed, 146 insertions(+), 36 deletions(-) create mode 100755 tests/bugs/compute-sum.sh diff --git a/src/language/xforms/ChangeLog b/src/language/xforms/ChangeLog index 19bcf233..4c79eeff 100644 --- a/src/language/xforms/ChangeLog +++ b/src/language/xforms/ChangeLog @@ -1,3 +1,19 @@ +Thu Nov 30 22:46:50 2006 Ben Pfaff + + * compute.c: Fix bug #17422, which reports that a variable created + by assignment in a COMPUTE command could not be used in the + computation expression. + (struct lvalue) Drop `var_name' member in favor of `variable' + pointer. Add `is_new_variable' member to allow us to know whether + to delete the variable at destruction time, in case of an error. + (lvalue_parse) Create variable whose name is specified, if it does + not already exist, as a numeric variable. + (lvalue_get_type) Rewrite to work with revised lvalue structure. + New `dict' parameter, which propagates back up its call chain. + (lvalue_finalize) No need to create variable here since we did so + at parse time. + (lvalue_destroy) Destroy variable if `is_new_variable' set. + Sat Oct 7 11:04:01 WST 2006 John Darrington * automake.mk fail.c: Added a debug transformation which always fails. diff --git a/src/language/xforms/compute.c b/src/language/xforms/compute.c index a29bb699..a2a7e7d2 100644 --- a/src/language/xforms/compute.c +++ b/src/language/xforms/compute.c @@ -44,11 +44,11 @@ struct lvalue; /* Target of a COMPUTE or IF assignment, either a variable or a vector element. */ static struct lvalue *lvalue_parse (struct lexer *lexer, struct dataset *); -static int lvalue_get_type (const struct lvalue *, const struct dictionary *); +static int lvalue_get_type (const struct lvalue *); static bool lvalue_is_vector (const struct lvalue *); static void lvalue_finalize (struct lvalue *, struct compute_trns *, struct dictionary *); -static void lvalue_destroy (struct lvalue *); +static void lvalue_destroy (struct lvalue *, struct dictionary *); /* COMPUTE and IF transformation. */ struct compute_trns @@ -74,7 +74,7 @@ static struct expression *parse_rvalue (struct lexer *lexer, struct dataset *); static struct compute_trns *compute_trns_create (void); -static trns_proc_func *get_proc_func (const struct lvalue *, const struct dictionary *); +static trns_proc_func *get_proc_func (const struct lvalue *); static trns_free_func compute_trns_free; /* COMPUTE. */ @@ -98,15 +98,14 @@ cmd_compute (struct lexer *lexer, struct dataset *ds) if (compute->rvalue == NULL) goto fail; - add_transformation (ds, get_proc_func (lvalue, dict), - compute_trns_free, compute); + add_transformation (ds, get_proc_func (lvalue), compute_trns_free, compute); lvalue_finalize (lvalue, compute, dict); return lex_end_of_command (lexer); fail: - lvalue_destroy (lvalue); + lvalue_destroy (lvalue, dict); compute_trns_free (compute); return CMD_CASCADING_FAILURE; } @@ -241,15 +240,14 @@ cmd_if (struct lexer *lexer, struct dataset *ds) if (compute->rvalue == NULL) goto fail; - add_transformation (ds, get_proc_func (lvalue, dict), - compute_trns_free, compute); + add_transformation (ds, get_proc_func (lvalue), compute_trns_free, compute); lvalue_finalize (lvalue, compute, dict); return lex_end_of_command (lexer); fail: - lvalue_destroy (lvalue); + lvalue_destroy (lvalue, dict); compute_trns_free (compute); return CMD_CASCADING_FAILURE; } @@ -257,9 +255,9 @@ cmd_if (struct lexer *lexer, struct dataset *ds) /* Code common to COMPUTE and IF. */ static trns_proc_func * -get_proc_func (const struct lvalue *lvalue, const struct dictionary *dict) +get_proc_func (const struct lvalue *lvalue) { - bool is_numeric = lvalue_get_type (lvalue, dict) == NUMERIC; + bool is_numeric = lvalue_get_type (lvalue) == NUMERIC; bool is_vector = lvalue_is_vector (lvalue); return (is_numeric @@ -273,8 +271,7 @@ static struct expression * parse_rvalue (struct lexer *lexer, const struct lvalue *lvalue, struct dataset *ds) { - const struct dictionary *dict = dataset_dict (ds); - bool is_numeric = lvalue_get_type (lvalue, dict) == NUMERIC; + bool is_numeric = lvalue_get_type (lvalue) == NUMERIC; return expr_parse (lexer, ds, is_numeric ? EXPR_NUMBER : EXPR_STRING); } @@ -308,10 +305,14 @@ compute_trns_free (void *compute_) return true; } -/* COMPUTE or IF target variable or vector element. */ +/* COMPUTE or IF target variable or vector element. + For a variable, the `variable' member is non-null. + For a vector element, the `vector' member is non-null. */ struct lvalue { - char var_name[LONG_NAME_LEN + 1]; /* Destination variable name, or "". */ + struct variable *variable; /* Destination variable. */ + bool is_new_variable; /* Did we create the variable? */ + const struct vector *vector; /* Destination vector, if any, or NULL. */ struct expression *element; /* Destination vector element, or NULL. */ }; @@ -321,10 +322,12 @@ struct lvalue static struct lvalue * lvalue_parse (struct lexer *lexer, struct dataset *ds) { + struct dictionary *dict = dataset_dict (ds); struct lvalue *lvalue; lvalue = xmalloc (sizeof *lvalue); - lvalue->var_name[0] = '\0'; + lvalue->variable = NULL; + lvalue->is_new_variable = false; lvalue->vector = NULL; lvalue->element = NULL; @@ -334,7 +337,7 @@ lvalue_parse (struct lexer *lexer, struct dataset *ds) if (lex_look_ahead (lexer) == '(') { /* Vector. */ - lvalue->vector = dict_lookup_vector (dataset_dict (ds), lex_tokid (lexer)); + lvalue->vector = dict_lookup_vector (dict, lex_tokid (lexer)); if (lvalue->vector == NULL) { msg (SE, _("There is no vector named %s."), lex_tokid (lexer)); @@ -354,31 +357,30 @@ lvalue_parse (struct lexer *lexer, struct dataset *ds) else { /* Variable name. */ - str_copy_trunc (lvalue->var_name, sizeof lvalue->var_name, lex_tokid (lexer)); + const char *var_name = lex_tokid (lexer); + lvalue->variable = dict_lookup_var (dict, var_name); + if (lvalue->variable == NULL) + { + lvalue->variable = dict_create_var_assert (dict, var_name, 0); + lvalue->is_new_variable = true; + } lex_get (lexer); } return lvalue; lossage: - lvalue_destroy (lvalue); + lvalue_destroy (lvalue, dict); return NULL; } /* Returns the type (NUMERIC or ALPHA) of the target variable or vector in LVALUE. */ static int -lvalue_get_type (const struct lvalue *lvalue, const struct dictionary *dict) +lvalue_get_type (const struct lvalue *lvalue) { - if (lvalue->vector == NULL) - { - struct variable *var = dict_lookup_var (dict, lvalue->var_name); - if (var == NULL) - return NUMERIC; - else - return var->type; - } - else - return lvalue->vector->var[0]->type; + return (lvalue->variable != NULL + ? lvalue->variable->type + : lvalue->vector->var[0]->type); } /* Returns true if LVALUE has a vector as its target. */ @@ -397,16 +399,16 @@ lvalue_finalize (struct lvalue *lvalue, { if (lvalue->vector == NULL) { - compute->variable = dict_lookup_var (dict, lvalue->var_name); - if (compute->variable == NULL) - compute->variable = dict_create_var_assert (dict, lvalue->var_name, 0); - + compute->variable = lvalue->variable; compute->fv = compute->variable->fv; compute->width = compute->variable->width; /* Goofy behavior, but compatible: Turn off LEAVE. */ if (dict_class_from_id (compute->variable->name) != DC_SCRATCH) compute->variable->leave = false; + + /* Prevent lvalue_destroy from deleting variable. */ + lvalue->is_new_variable = false; } else { @@ -415,16 +417,18 @@ lvalue_finalize (struct lvalue *lvalue, lvalue->element = NULL; } - lvalue_destroy (lvalue); + lvalue_destroy (lvalue, dict); } /* Destroys LVALUE. */ static void -lvalue_destroy (struct lvalue *lvalue) +lvalue_destroy (struct lvalue *lvalue, struct dictionary *dict) { if (lvalue == NULL) return; + if (lvalue->is_new_variable) + dict_delete_var (dict, lvalue->variable); expr_free (lvalue->element); free (lvalue); } diff --git a/tests/ChangeLog b/tests/ChangeLog index e6696c45..66d0f034 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,9 @@ +Thu Nov 30 22:46:17 2006 Ben Pfaff + + * automake.mk: Add new test. + + * tests/bugs/compute-sum.sh: New test, for bug #17422. + Thu Nov 30 22:01:57 2006 Ben Pfaff * automake.mk: Add new test. diff --git a/tests/automake.mk b/tests/automake.mk index 54d7c326..fa5ff988 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -109,6 +109,7 @@ TESTS = \ tests/bugs/recode-copy-bug.sh \ tests/bugs/computebug.sh \ tests/bugs/compute-lv.sh \ + tests/bugs/compute-sum.sh \ tests/bugs/temp-freq.sh \ tests/bugs/print-crash.sh \ tests/bugs/keep-all.sh \ diff --git a/tests/bugs/compute-sum.sh b/tests/bugs/compute-sum.sh new file mode 100755 index 00000000..4b043a92 --- /dev/null +++ b/tests/bugs/compute-sum.sh @@ -0,0 +1,83 @@ +#!/bin/sh + +# This program tests for a bug in the `compute' command, in which it +# failed to allow a newly created variable to be used as part of the +# computation, which actually makes sense for "LEAVE" variables. + +TEMPDIR=/tmp/pspp-tst-$$ + +# ensure that top_builddir are absolute +if [ -z "$top_builddir" ] ; then top_builddir=. ; fi +if [ -z "$top_srcdir" ] ; then top_srcdir=. ; fi +top_builddir=`cd $top_builddir; pwd` +PSPP=$top_builddir/src/ui/terminal/pspp + +# ensure that top_srcdir is absolute +top_srcdir=`cd $top_srcdir; pwd` + +STAT_CONFIG_PATH=$top_srcdir/config +export STAT_CONFIG_PATH + + +cleanup() +{ + cd / + rm -rf $TEMPDIR +} + + +fail() +{ + echo $activity + echo FAILED + cleanup; + exit 1; +} + + +no_result() +{ + echo $activity + echo NO RESULT; + cleanup; + exit 2; +} + +pass() +{ + cleanup; + exit 0; +} + +mkdir -p $TEMPDIR +cd $TEMPDIR + +activity="copy file" +cat > compute-sum.stat <