* 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.

diff --git a/src/language/xforms/ChangeLog b/src/language/xforms/ChangeLog
index 19bcf2336c938d090c9901a37079394df4fb4990..4c79eeff4e279e4749c5df1e6624cb2c83c295d2 100644 (file)
--- a/src/language/xforms/ChangeLog
+++ b/src/language/xforms/ChangeLog
@@ -1,3 +1,19 @@
+Thu Nov 30 22:46:50 2006  Ben Pfaff  <blp@gnu.org>
+
+       * 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 <john@darrington.wattle.id.au>
 
        * 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 a29bb699db7c37778df5708fe9443aff3828390c..a2a7e7d2a430c108cae7170888ebabea2f7fafd8 100644 (file)
--- 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;
 \f
 /* 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;
 }
 \f
-/* 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 e6696c452d938614987e30f9c2a4a75629827857..66d0f0347bf64d6e4aff3ff5bfb7bbf79e675a1a 100644 (file)
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,9 @@
+Thu Nov 30 22:46:17 2006  Ben Pfaff  <blp@gnu.org>
+
+       * automake.mk: Add new test.
+
+       * tests/bugs/compute-sum.sh: New test, for bug #17422.
+
 Thu Nov 30 22:01:57 2006  Ben Pfaff  <blp@gnu.org>
 
        * automake.mk: Add new test.

diff --git a/tests/automake.mk b/tests/automake.mk
index 54d7c326228a92553931d508ff15f9208e9c843b..fa5ff988ed1162ef76e68c98239f41b14539c4ef 100644 (file)
--- 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 (executable)
index 0000000..4b043a9
--- /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 <<EOF
+DATA LIST /ITEM 1-3.
+COMPUTE SUM=SUM+ITEM.
+PRINT OUTFILE='compute-sum.out' /ITEM SUM.
+LEAVE SUM
+BEGIN DATA.
+123
+404
+555
+999
+END DATA.
+EOF
+if [ $? -ne 0 ] ; then no_result ; fi
+
+activity="run program"
+$SUPERVISOR $PSPP $TEMPDIR/compute-sum.stat
+if [ $? -ne 0 ] ; then no_result ; fi
+
+activity="compare output"
+diff compute-sum.out - <<EOF
+ 123   123.00 
+ 404   527.00 
+ 555  1082.00 
+ 999  2081.00 
+EOF
+if [ $? -ne 0 ] ; then fail ; fi
+
+pass;