From octave-maintainers-request at bevo dot che dot wisc dot edu Wed Jan  1 21:46:12 2003
Subject: Re: f(end) feature?
From: "John W. Eaton" <jwe at bevo dot che dot wisc dot edu>
To: "John W. Eaton" <jwe at bevo dot che dot wisc dot edu>
Cc: octave-maintainers at bevo dot che dot wisc dot edu
Date: Wed, 1 Jan 2003 21:45:47 -0600

On 31-Dec-2002, John W. Eaton <jwe at bevo dot che dot wisc dot edu> wrote:

| On 31-Dec-2002, Paul Kienzle <pkienzle at jazz dot ncnr dot nist dot gov> wrote:
| 
| | On Tue, Dec 31, 2002 at 04:31:48PM -0600, John W. Eaton wrote:
| | > 
| | > Since we save and restore the global pointer with the unwind protect
| | > stack, it should work correctly for recursion to any depth.
| | 
| | So we pay for the feature even if we are not using it?  It's worth
| | it as far as I'm concerned, and perhaps even cheaper than doing
| | a call to a builtin function like length() or size().
| 
| Yes, but at least it works.  The cost is:
| 
|   Pass a pointer in each call to tree_argument_list::convert_to_const_vector
|   even if it is not needed.
| 
|   In convert_to_const_vector, do
| 
|     bool stash_object = (object && object->is_constant ());
| 
|     if (stash_object)
|       {
| 	unwind_protect::begin_frame ("convert_to_const_vector");
| 
| 	unwind_protect_ptr (indexed_object);
| 
| 	indexed_object = object;
|       }
| 
|   at the beginning of the function,
| 
|     index_position = (len == 1) ? -1 : k;
| 
|   for each element of the argument list, and then
| 
|     if (stash_object)
|       unwind_protect::run_frame ("convert_to_const_vector");
| 
|   at the end of the function.
| 
| Most of the overhead is in handling the unwind_protect stack.  To do
| better, we could try to mark expressions that might actually need to
| have the global variables set, then only do the save/restore for those
| expressions that have been marked.  This would be similar to your idea
| of modifying the parse tree to insert some code where needed.  I'll
| see whether that would be easy to do.

OK, it was not hard.  Here is the patch.

jwe


src/ChangeLog:

2003-01-01  John W. Eaton  <jwe at bevo dot che dot wisc dot edu>

	* pt-arg-list.cc (tree_argument_list::append): New function.
	(tree_argument_list::convert_to_const_vector): Don't save and
	set pointer to indexed object if list_includes_magic_end is false.
	* pt-arg-list.h (tree_argument_list::append): Provide decl.
	(tree_argument_list::list_includes_magic_end): New data member.
	(tree_argument_list::tree_argument_list): Initialize it.


Index: pt-arg-list.h
===================================================================
RCS file: /usr/local/cvsroot/octave/src/pt-arg-list.h,v
retrieving revision 1.9
diff -u -r1.9 pt-arg-list.h
--- pt-arg-list.h	21 Dec 2002 17:15:25 -0000	1.9
+++ pt-arg-list.h	2 Jan 2003 03:32:13 -0000
 at @ -45,9 +45,13 @@
 {
 public:
 
-  tree_argument_list (void) { }
+  typedef tree_expression* element_type;
 
-  tree_argument_list (tree_expression *t) { append (t); }
+  tree_argument_list (void)
+    : list_includes_magic_end (false) { }
+
+  tree_argument_list (tree_expression *t)
+    : list_includes_magic_end (false) { append (t); }
 
   ~tree_argument_list (void);
 
 at @ -59,6 +63,8 @@
       return retval;
     }
 
+  void append (const element_type& s);
+
   int nargout_count (void) const;
 
   bool all_elements_are_constant (void) const;
 at @ -70,6 +76,8 @@
   void accept (tree_walker& tw);
 
 private:
+
+  bool list_includes_magic_end;
 
   // No copying!

Index: pt-arg-list.cc
===================================================================
RCS file: /usr/local/cvsroot/octave/src/pt-arg-list.cc,v
retrieving revision 1.14
diff -u -r1.14 pt-arg-list.cc
--- pt-arg-list.cc	31 Dec 2002 21:48:54 -0000	1.14
+++ pt-arg-list.cc	2 Jan 2003 03:32:13 -0000
 at @ -57,6 +57,15 @@
     }
 }
 
+void
+tree_argument_list::append (const element_type& s)
+{
+  octave_base_list<tree_expression *>::append (s);
+
+  if (s && s->is_identifier () && s->name () == "__end__")
+    list_includes_magic_end = true;
+}
+
 int
 tree_argument_list::nargout_count (void) const
 {
 at @ -154,7 +163,8 @@
   // END doesn't make sense for functions.  Maybe we need a different
   // way of asking an octave_value object this question?
 
-  bool stash_object = (object && object->is_constant ());
+  bool stash_object = (list_includes_magic_end
+		       && object && object->is_constant ());
 
   if (stash_object)
     {