Proposed patch for external debugger access

[John Swensen]

I have attached a patch to address some of the issues with IDE's that 
are going to want access to debugger information.  John Eaton had 
suggested that in the future the method of breakpoint storage and the 
location of such information may change, and this tries to provide a 
simple interface that abstracts the underlying implementation in terms 
of the parser and symbol information, away from the user API.  This 
version changed very little functionality, and the ways in which it did 
make it more similar to Matlab.

Synopsis of Changes:
1) Created a class called bp_table which provides: add_breakpoint, 
remove_breakpoint, remove_all_breakpoints, get_breakpoint_list, etc and 
2 "helper" functions
2) Changes the DEFCMD functions for dbstop, dbclear, and dbstatus 
(dbwhere and dbtype were left alone) to use the newly created class
3) Made the output of dbstatus more like Matlab (e.g. print when 
nargout=0 and output a struct when nargout=1).  Also, when not in debug 
mode, a call to dbstatus with no parameters will give a complete list of 
breakpoints.

Now, a little more about the bp_table class.  In its current 
implementation, it simply maintains an STL map of the pair <string 
function_name, octave_user_function*>.  Let me know what you think.

John Swensen

P.S. JWE, I still have the hardest time with your desired paren spacing 
;)  I do see your argument about it being more like the English 
language, it is just that I am so used to doing it the other way that it 
still seems awkward in terms of typing where I don't have that muscle 
memory yet and I actually have to pause and force myself to do it.  
However, I do think I caught all of them in this patch.
--- /dev/null   2007-10-23 19:38:27.000000000 -0400
+++ debug.h     2007-10-23 19:34:41.000000000 -0400
@@ -0,0 +1,138 @@
+/*
+
+Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006, 2007 Ben Sapp, 
+              2007 John Swensen
+
+This file is part of Octave.
+
+Octave is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by the
+Free Software Foundation; either version 3 of the License, or (at your
+option) any later version.
+
+Octave is distributed in the hope that it will be useful, but WITHOUT
+ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with Octave; see the file COPYING.  If not, see
+<http://www.gnu.org/licenses/>.
+
+*/
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <iostream>
+#include <fstream>
+#include <string>
+#include <map>
+
+#include "defun.h"
+#include "error.h"
+#include "input.h"
+#include "pager.h"
+#include "oct-obj.h"
+#include "utils.h"
+#include "parse.h"
+#include "symtab.h"
+#include "gripes.h"
+#include "ov.h"
+#include "ov-usr-fcn.h"
+#include "ov-fcn.h"
+#include "ov-struct.h"
+#include "pt-pr-code.h"
+#include "pt.h"
+#include "pt-bp.h"
+#include "pt-stmt.h"
+#include "toplev.h"
+#include "unwind-prot.h"
+#include "variables.h"
+
+/**
+ * A class to provide a standard interface to breakpoints, even if the 
associated backend
+ * changes, we can make sure this higher level interface stays the same.
+ */
+class bp_table
+{
+public:
+  /**
+   * Default constructor
+   */
+  bp_table ();
+
+  /**
+   * Destructor
+   */
+  ~bp_table ();
+
+  /**
+   * Adds a breakpoint at the nearest executable line.
+   *
+   * @param fname The name of the m-file in which to set the breakpoint
+   * @param line The line number on which to set the breakpoint.
+   *
+   * @return Returns the line number at which the breakpoint was
+   * actually placed. This is only different than the input if the
+   * input was not an executable line.
+   */
+  octave_value_list add_breakpoint (std::string fname = "", octave_value_list 
lines = octave_value_list());
+
+  /**
+   * Remove a breakpoint from a line in file.  If the line specified
+   * is not an executable line, it will increment to the next executable
+   * line.
+   *
+   * @param fname The name of the m-file in which to clear the breakpoint.
+   * @param line The line number on which the breakpoint to be cleared resides.
+   *
+   * @return Returns the line number on which the cleared breakpoint resided
+   * or -1 otherwise.
+   */
+  int remove_breakpoint (std::string fname = "", octave_value_list lines = 
octave_value_list());
+
+  /**
+   * Removes all the breakpoints in a specified file.
+   *
+   * @param fname The name of the m-file from which to clear all breakpoints
+   *
+   * @return A vector of the line number from which breakpoints were removed.
+   */
+  octave_value_list remove_all_breakpoints_in_file (std::string fname);
+  
+
+  /**
+   * Removes all the breakpoints registered with octave.
+   *
+   * @return A complete list of all removed breakpoints.
+   */
+  void remove_all_breakpoints (void);
+  
+  /**
+   * Returns a list of breakpoints in the system.  In the absence of the 
parameter, all
+   * breakpoints are returned.  Using the parameter, a subset of the 
breakpoints can be
+   * queried.
+   *
+   * @param fname_list A list of function names for which to check for 
breakpoints
+   *
+   * @return A map of the pairwise <file, linenumber>
+   */
+  std::map< std::string, octave_value_list > get_breakpoint_list ( 
octave_value_list fname_list );
+
+private:
+
+  /**
+   * The STL map containing a list of the pairwise <filename, 
octave_user_function*> for
+   * every function that contains at least one breakpoint.
+   */
+  std::map< std::string, octave_user_function* > bp_map;
+};
+
+extern bp_table breakpoints;
+
+/*
+;;; Local Variables: ***
+;;; mode: C++ ***
+;;; End: ***
+*/

Index: debug.cc
===================================================================
RCS file: /cvs/octave/src/debug.cc,v
retrieving revision 1.26
diff -u -r1.26 debug.cc
--- debug.cc    12 Oct 2007 21:27:29 -0000      1.26
+++ debug.cc    23 Oct 2007 23:38:33 -0000
@@ -1,6 +1,7 @@
 /*
 
-Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006, 2007 Ben Sapp
+Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006, 2007 Ben Sapp,
+              2007 John Swensen
 
 This file is part of Octave.
 
@@ -19,39 +20,14 @@
 <http://www.gnu.org/licenses/>.
 
 */
+#include "debug.h"
 
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <iostream>
-#include <fstream>
-#include <string>
-
-#include "defun.h"
-#include "error.h"
-#include "input.h"
-#include "pager.h"
-#include "oct-obj.h"
-#include "utils.h"
-#include "parse.h"
-#include "symtab.h"
-#include "gripes.h"
-#include "ov.h"
-#include "ov-usr-fcn.h"
-#include "ov-fcn.h"
-#include "pt-pr-code.h"
-#include "pt.h"
-#include "pt-bp.h"
-#include "pt-stmt.h"
-#include "toplev.h"
-#include "unwind-prot.h"
-#include "variables.h"
+// Global variable containing the breakpoint information
+bp_table breakpoints;
 
 // Return a pointer to the user-defined function FNAME.  If FNAME is
 // empty, search backward for the first user-defined function in the
 // current call stack.
-
 static octave_user_function *
 get_user_function (std::string fname = "")
 {
@@ -83,98 +59,216 @@
   return dbg_fcn;
 }
 
-
-DEFCMD (dbstop, args, ,
-  "-*- texinfo -*-\n\
address@hidden {Loadable Function} {rline =} dbstop (func, line, @dots{})\n\
-Set a breakpoint in a function\n\
address@hidden @code\n\
address@hidden func\n\
-String representing the function name.  When already in debug\n\
-mode this should be left out and only the line should be given.\n\
address@hidden line\n\
-Line you would like the breakpoint to be set on. Multiple\n\
-lines might be given as separate arguments or as a vector.\n\
address@hidden table\n\
-\n\
-The rline returned is the real line that the breakpoint was set at.\n\
address@hidden, dbstatus, dbnext}\n\
address@hidden deftypefn")
+/**
+ * Parse a set of function arguments and return the name of the function and
+ * the desired line numbers.  It expects the first argument to be a function
+ * name if not currently in a debug function, otherwise is begins parsing the
+ * line numbers from the beginning.
+ *
+ * @param The list of function arguments.
+ * @param symbol_name Reference variable in which the symbol_name is returned.
+ * @param lines Reference variable in which the line numbers are returned.
+ */
+static void
+parse_dbfunction_params ( octave_value_list args, 
+                         std::string& symbol_name, octave_value_list& lines )
 {
-  octave_value retval;
   int nargin = args.length ();
   int idx = 0;
-  std::string symbol_name = "";
-
-  if (nargin != 1 && args(0).is_string())
+  int list_idx = 0;
+  symbol_name = std::string();
+  lines = octave_value_list();
+
+  // Build up a list of line numbers with the associated
+  /// If we are already in a debug function, then just interpret as line 
numbers
+  if (octave_call_stack::caller_user_function () != NULL)
+    {
+      idx = 0;
+    }
+  else
     {
       symbol_name = args(0).string_value ();
       idx = 1;
     }
 
-  octave_user_function *dbg_fcn = get_user_function (symbol_name);
-
-  if (dbg_fcn)
+  for (int i = idx; i < nargin ; i++ )
     {
-      octave_idx_type nsize = 10;
-      RowVector results (nsize);
-      octave_idx_type nr = 0;
-
-      tree_statement_list *cmds = dbg_fcn->body ();
-
-      for (int i = idx; i < nargin; i++)
+      if (args(i).is_string())
+       {
+         int line = atoi (args(i).string_value ().c_str ());
+         if (error_state)
+             break;
+         lines(list_idx++) = octave_value(line);
+       }
+      else
        {
-         if (args(i).is_string ())
+         const NDArray arg = args(i).array_value ();
+         
+         if (error_state)
+           break;
+         
+         for (octave_idx_type j = 0; j < arg.nelem(); j++)
            {
-             int line = atoi (args(i).string_value ().c_str ());
-
+             int line = static_cast<int> (arg.elem (j));
              if (error_state)
                break;
+             lines(list_idx++) = octave_value(line);
+           }
+         
+         if (error_state)
+           break;
+       }
+    } 
+}
 
-             if (nr == nsize)
-               {
-                 nsize *= 2;
-                 results.resize (nsize);
-               }
 
-             results(nr++) = cmds->set_breakpoint (line);
-           }
-         else
-           {
-             const NDArray arg = args(i).array_value ();
+//-----------------------------------------------------------------------------------
+bp_table::bp_table ()
+{
+ 
+}
 
-             if (error_state)
-               break;
+//-----------------------------------------------------------------------------------
+bp_table::~bp_table ()
+{
 
-             for (octave_idx_type j = 0; j < arg.nelem(); j++)
-               {
-                 int line = static_cast<int> (arg.elem (j));
+}
 
-                 if (error_state)
-                   break;
+//-----------------------------------------------------------------------------------
+octave_value_list bp_table::add_breakpoint (std::string fname, 
octave_value_list line )
+{
+  octave_value_list retval;
+  octave_user_function *dbg_fcn = get_user_function (fname);
 
-                 if (nr == nsize)
-                   {
-                     nsize *= 2;
-                     results.resize (nsize);
-                   }
+  if (dbg_fcn)
+    {
+      tree_statement_list *cmds = dbg_fcn->body ();
 
-                 results(nr++) = cmds->set_breakpoint (line);
-               }
+      for (int i = 0; i < line.length(); i++)
+       {
+         int lineno = static_cast<int> (line(i).matrix_value()(0));
+         retval(i) = octave_value (cmds->set_breakpoint (lineno));
+         if (static_cast<int> (retval(i).matrix_value()(0)) != 0)
+           bp_map[fname] = dbg_fcn;
+       }
+    }
+  else
+    error ("add_breakpoint: unable to find the function requested\n");
 
-             if (error_state)
-               break;
-           }
+  return retval;
+}
+
+//-----------------------------------------------------------------------------------
+int bp_table::remove_breakpoint (std::string fname, octave_value_list line)
+{
+  octave_value_list retval;
+  octave_user_function *dbg_fcn = get_user_function (fname);
+
+  if (dbg_fcn)
+    {
+      tree_statement_list *cmds = dbg_fcn->body ();
+      for (int i = 0; i < line.length(); i++)
+       {
+         cmds->delete_breakpoint ( static_cast<int> (line(i).matrix_value 
()(0)));
        }
+      retval = cmds->list_breakpoints ();
+      if ( retval.length () == 0)
+       bp_map.erase (bp_map.find (fname) );
+    }
+  else
+    error ("remove_breakpoint: unable to find the function requested\n");
+
+  return retval.length();
+}
 
-      if (! error_state)
+//-----------------------------------------------------------------------------------
+octave_value_list bp_table::remove_all_breakpoints_in_file (std::string fname)
+{
+  octave_value_list bkpts;
+  octave_user_function *dbg_fcn = get_user_function (fname);
+  
+  if (dbg_fcn)
+    {
+      tree_statement_list *cmds = dbg_fcn->body ();
+      bkpts = cmds->list_breakpoints();
+      for (int i = 0; i < bkpts.length(); i++)
        {
-         results.resize (nr);
-         retval = results;
+         cmds->delete_breakpoint (static_cast<int> (bkpts(i).matrix_value 
()(0)));
        }
+      bp_map.erase (bp_map.find (fname) );
     }
   else
-    error ("dbstop: unable to find the function requested\n");
+    error ("remove_all_breakpoint_in_file: unable to find the function 
requested\n");
+
+  return bkpts;
+}
+
+//-----------------------------------------------------------------------------------
+void bp_table::remove_all_breakpoints (void)
+{
+  std::map< std::string, octave_user_function* >::iterator it;
+  for ( it = bp_map.begin () ; it != bp_map.end () ; it++ )
+    {
+      remove_all_breakpoints_in_file( (*it).first );
+    }
+}
+
+std::string do_find_bkpt_list (octave_value_list slist, std::string match )
+{
+  std::string retval;
+  for( int i = 0 ; i < slist.length() ; i++ )
+    {
+      if (slist(i).string_value() == match)
+       {
+         retval = slist(i).string_value ();
+         break;
+       }
+    }
+  return retval;
+}
+
+//-----------------------------------------------------------------------------------
+std::map< std::string, octave_value_list > bp_table::get_breakpoint_list ( 
octave_value_list fname_list )
+{
+  std::map<std::string,octave_value_list> retval;
+
+  // Iterate through each of the files in the map and get the name and list of 
breakpoints
+  std::map< std::string, octave_user_function* >::iterator it;
+  for (it = bp_map.begin(); it != bp_map.end() ; it++ )
+    {
+      if( fname_list.length() == 0 || do_find_bkpt_list (fname_list, 
(*it).first) != "" )
+       {
+         octave_value_list bkpts = (*it).second->body()->list_breakpoints();
+         retval[ (*it).first ] = bkpts;
+       }
+    }
+  return retval;
+}
+
+DEFCMD (dbstop, args, ,
+  "-*- texinfo -*-\n\
address@hidden {Loadable Function} {rline =} dbstop (func, line, @dots{})\n\
+Set a breakpoint in a function\n\
address@hidden @code\n\
address@hidden func\n\
+String representing the function name.  When already in debug\n\
+mode this should be left out and only the line should be given.\n\
address@hidden line\n\
+Line you would like the breakpoint to be set on. Multiple\n\
+lines might be given as separate arguments or as a vector.\n\
address@hidden table\n\
+\n\
+The rline returned is the real line that the breakpoint was set at.\n\
address@hidden, dbstatus, dbnext}\n\
address@hidden deftypefn")
+{
+  octave_value retval;
+  std::string symbol_name = "";
+  octave_value_list lines;
+  parse_dbfunction_params (args, symbol_name, lines);
+  
+  if (!error_state)
+    retval = breakpoints.add_breakpoint( symbol_name, lines );
 
   return retval;
 }
@@ -197,62 +291,17 @@
 @end deftypefn")
 {
   octave_value retval;
-  int nargin = args.length ();
-  int idx = 0;
   std::string symbol_name = "";
-
-  if (nargin != 1 && args(0).is_string())
-    {
-      symbol_name = args(0).string_value ();
-      idx = 1;
-    }
-
-  octave_user_function *dbg_fcn = get_user_function (symbol_name);
-
-  if (dbg_fcn)
-    {
-      tree_statement_list *cmds = dbg_fcn->body ();
-
-      for (int i = idx; i < nargin; i++)
-       {
-         if (args(i).is_string ())
-           {
-             int line = atoi (args(i).string_value ().c_str ());
-
-             if (error_state)
-               break;
-
-             cmds->delete_breakpoint (line);
-           }
-         else
-           {
-             const NDArray arg = args(i).array_value ();
-
-             if (error_state)
-               break;
-
-             for (octave_idx_type j = 0; j < arg.nelem (); j++)
-               {
-                 int line = static_cast<int> (arg.elem (j));
-
-                 if (error_state)
-                   break;
-
-                 cmds->delete_breakpoint (line);
-               }
-
-             if (error_state)
-               break;
-           }
-       }
-    }
-  else
-    error ("dbclear: unable to find the function requested\n");
-
+  octave_value_list lines;
+  parse_dbfunction_params (args, symbol_name, lines);
+      
+  if (!error_state)
+    breakpoints.remove_breakpoint( symbol_name, lines );
+      
   return retval;
 }
 
-DEFCMD (dbstatus, args, ,
+DEFCMD (dbstatus, args, nargout,
   "-*- texinfo -*-\n\
 @deftypefn {Loadable Function} {lst =} dbstatus ([func])\n\
 Return a vector containing the lines on which a function has \n\
@@ -265,50 +314,72 @@
 @seealso{dbclear, dbwhere}\n\
 @end deftypefn")
 {
-  octave_value retval;
-
+  Octave_map retval;
   int nargin = args.length ();
+  octave_value_list fcn_list;
+  std::map< std::string, octave_value_list > bp_list;
+  std::string symbol_name = "";
 
   if (nargin != 0 && nargin != 1)
     {
       error ("dbstatus: only zero or one arguements accepted\n");
-      return retval;
+      return octave_value(retval);
     }
 
-  std::string symbol_name = "";
-
   if (nargin == 1)
     {
       if (args(0).is_string ())
-       symbol_name = args(0).string_value ();
+       {
+         symbol_name = args(0).string_value ();
+         fcn_list(0) = symbol_name;
+         bp_list = breakpoints.get_breakpoint_list(fcn_list);
+       }
       else
        gripe_wrong_type_arg ("dbstatus", args(0));
     }
-
-  octave_user_function *dbg_fcn = get_user_function (symbol_name);
-
-  if (dbg_fcn)
+  else
     {
-      tree_statement_list *cmds = dbg_fcn->body ();
-
-      octave_value_list lst = cmds->list_breakpoints ();
-
-      RowVector vec (lst.length (), 0.0);
-
-      for (int i = 0; i < lst.length (); i++)
+       octave_user_function *dbg_fcn = get_user_function ();
+       if (dbg_fcn)
+        {
+          symbol_name = dbg_fcn->name();
+          fcn_list(0) = symbol_name;
+        }
+       bp_list = breakpoints.get_breakpoint_list(fcn_list);
+    }
+
+  std::map< std::string, octave_value_list >::iterator it;
+  if (nargout == 1)
+    {
+      // Fill in an array for return
+      int i = 0;
+      Cell namesCell (dim_vector (bp_list.size(), 1));
+      Cell lineCell (dim_vector (bp_list.size(), 1));
+      for ( it = bp_list.begin() ; it != bp_list.end() ; it++ )
        {
-         vec(i) = lst(i).double_value ();
-
-         if (error_state)
-           panic_impossible ();
+         namesCell(i) = (*it).first;
+         lineCell(i)  = (*it).second;
+         i++;
        }
-
-      retval = octave_value (vec);
+      retval.assign( "name", namesCell );
+      retval.assign( "line", lineCell );
     }
   else
-    error ("dbstatus: unable to find the function you requested\n");
+    {
+      // Print out the breakpoint information
+      
+      for ( it = bp_list.begin() ; it != bp_list.end() ; it++ )
+       {         
+         octave_stdout << "Breakpoint in " << (*it).first << " at line(s) ";
+         for( int j = 0 ; j < (*it).second.length() ; j++ )
+           if( j < (*it).second.length()-1 )
+             octave_stdout << (*it).second(j).int_value() << ", ";
+           else
+             octave_stdout << (*it).second(j).int_value() << "." << std::endl;
+       }
+    }
+  return octave_value(retval);
 
-  return retval;
 }
 
 DEFCMD (dbwhere, , ,