[Pspp-cvs] pspp src/language/xforms/ChangeLog src/language...

[Ben Pfaff]

CVSROOT:        /cvsroot/pspp
Module name:    pspp
Changes by:     Ben Pfaff <blp> 06/12/01 06:51:08

Modified files:
        src/language/xforms: ChangeLog compute.c 
        tests          : ChangeLog automake.mk 
Added files:
        tests/bugs     : compute-sum.sh 

Log message:
                * 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.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/pspp/src/language/xforms/ChangeLog?cvsroot=pspp&r1=1.6&r2=1.7
http://cvs.savannah.gnu.org/viewcvs/pspp/src/language/xforms/compute.c?cvsroot=pspp&r1=1.10&r2=1.11
http://cvs.savannah.gnu.org/viewcvs/pspp/tests/ChangeLog?cvsroot=pspp&r1=1.68&r2=1.69
http://cvs.savannah.gnu.org/viewcvs/pspp/tests/automake.mk?cvsroot=pspp&r1=1.19&r2=1.20
http://cvs.savannah.gnu.org/viewcvs/pspp/tests/bugs/compute-sum.sh?cvsroot=pspp&rev=1.1

Patches:
Index: src/language/xforms/ChangeLog
===================================================================
RCS file: /cvsroot/pspp/pspp/src/language/xforms/ChangeLog,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -b -r1.6 -r1.7
--- src/language/xforms/ChangeLog       7 Oct 2006 03:10:14 -0000       1.6
+++ src/language/xforms/ChangeLog       1 Dec 2006 06:51:08 -0000       1.7
@@ -1,3 +1,19 @@
+Thu Nov 30 22:46:50 2006  Ben Pfaff  <address@hidden>
+
+       * 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 <address@hidden>
 
        * automake.mk fail.c: Added a debug transformation which always fails.

Index: src/language/xforms/compute.c
===================================================================
RCS file: /cvsroot/pspp/pspp/src/language/xforms/compute.c,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -b -r1.10 -r1.11
--- src/language/xforms/compute.c       11 Nov 2006 23:10:01 -0000      1.10
+++ src/language/xforms/compute.c       1 Dec 2006 06:51:08 -0000       1.11
@@ -44,11 +44,11 @@
 /* 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 @@
                                        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 @@
   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 @@
   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 @@
 /* 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 @@
 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 @@
   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 @@
 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 @@
   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 @@
   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 @@
 {
   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->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);
 }

Index: tests/ChangeLog
===================================================================
RCS file: /cvsroot/pspp/pspp/tests/ChangeLog,v
retrieving revision 1.68
retrieving revision 1.69
diff -u -b -r1.68 -r1.69
--- tests/ChangeLog     1 Dec 2006 06:02:35 -0000       1.68
+++ tests/ChangeLog     1 Dec 2006 06:51:08 -0000       1.69
@@ -1,3 +1,9 @@
+Thu Nov 30 22:46:17 2006  Ben Pfaff  <address@hidden>
+
+       * automake.mk: Add new test.
+
+       * tests/bugs/compute-sum.sh: New test, for bug #17422.
+
 Thu Nov 30 22:01:57 2006  Ben Pfaff  <address@hidden>
 
        * automake.mk: Add new test.

Index: tests/automake.mk
===================================================================
RCS file: /cvsroot/pspp/pspp/tests/automake.mk,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -b -r1.19 -r1.20
--- tests/automake.mk   1 Dec 2006 06:02:35 -0000       1.19
+++ tests/automake.mk   1 Dec 2006 06:51:08 -0000       1.20
@@ -109,6 +109,7 @@
        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 \

Index: tests/bugs/compute-sum.sh
===================================================================
RCS file: tests/bugs/compute-sum.sh
diff -N tests/bugs/compute-sum.sh
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/bugs/compute-sum.sh   1 Dec 2006 06:51:08 -0000       1.1
@@ -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;