Re: Proposed patch for external debugger access

[John Swensen]

John W. Eaton wrote:
> On 24-Oct-2007, John Swensen wrote:
>
> | +  if (sym_rec && sym_rec->is_defined ())
> | +    retval = sym_rec->which ();
> | +  else
> | +    retval = fcn_file_in_path (name);
>
> This stuff is going to break when the object branch is merged.  I
> don't think there's anything for you to do about that now, but I guess
> it is worth mentioning it anyway.  If you have significant changes
> that involve the symbol table, it might be best to defer them until
> after 3.0 or to work on the object branch instead.
>
>   
I simply copied this function from the help.cc file.  Since it was 
declared "static" in the help.cc file, I could not access it from 
debug.cc.  Is it possible to make this do_which() function available 
from outside help.cc?  Then I wouldn't need to replicate it in debug.cc 
and any changes to do_which() for the object branch would follow 
directly in debug.cc.
>   // Return all breakpoints.  Each element of the map is a vector
>   // containing the breakpoints corresponding to a given function name.
>
> ?  What will happen when we have function overloading and there may be
> more than one function with the same name?  Or is this name an
> absolute file name?
>
>   
I'm not 100% sure about how to handle this one.  Currently in Matlab, if 
you set a breakpoint using dbstop, it will place the breakpoint in 
whichever file occurs first in the search path.  In fact, if you try to 
add a breakpoint via their editor, it asks whether you want to add the 
directory to the top of the search path or change the execution path to 
the directory containing the file.  I'm not exactly sure how the 
function overloading will work, so I guess I can't comment yet.  When a 
user calls 's=dbstatus()' it will return the full path of the file in 
which the breakpoint was placed in one of the structure fields.

I think I addressed all of your issues, with the exception of the couple 
of comments above.  Attached are the patches with the updates.

John Swensen

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    29 Oct 2007 19:30:44 -0000
@@ -19,7 +19,6 @@
 <http://www.gnu.org/licenses/>.
 
 */
-
 #ifdef HAVE_CONFIG_H
 #include <config.h>
 #endif
@@ -27,6 +26,8 @@
 #include <iostream>
 #include <fstream>
 #include <string>
+#include <set>
+
 
 #include "defun.h"
 #include "error.h"
@@ -40,6 +41,8 @@
 #include "ov.h"
 #include "ov-usr-fcn.h"
 #include "ov-fcn.h"
+#include "ov-list.h"
+#include "ov-struct.h"
 #include "pt-pr-code.h"
 #include "pt.h"
 #include "pt-bp.h"
@@ -48,10 +51,14 @@
 #include "unwind-prot.h"
 #include "variables.h"
 
+#include "debug.h"
+
+// Table of breakpoints
+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,102 +90,265 @@
   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")
+static void
+parse_dbfunction_params (const octave_value_list& args, 
+                        std::string& symbol_name, 
+                        intmap& lines)
 {
-  octave_value retval;
+  octave_idx_type len = 0;
   int nargin = args.length ();
   int idx = 0;
-  std::string symbol_name = "";
+  int list_idx = 0;
+  symbol_name = std::string ();
 
-  if (nargin != 1 && args(0).is_string())
+  // If we are already in a debugging function
+  if (octave_call_stack::caller_user_function () != NULL)
     {
-      symbol_name = args(0).string_value ();
+      idx = 0;
+    }
+  else
+    {
+      symbol_name = args (0).string_value ();
+      if (error_state)
+       return;
       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 ();
+      if (args (i).is_string ())
+       len += 1;
+      else
+       len += args (i).numel ();
+    }
 
-      for (int i = idx; i < nargin; i++)
+  lines = intmap();
+  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++] = 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++] = line;
+           }
+         
+         if (error_state)
+           break;
+       }
+    } 
+}
 
-             if (nr == nsize)
-               {
-                 nsize *= 2;
-                 results.resize (nsize);
-               }
+// copied from help.cc; it is static there so "unavailable" elsewhere
+static std::string
+do_which (const std::string& name)
+{
+  std::string retval;
 
-             results(nr++) = cmds->set_breakpoint (line);
-           }
-         else
-           {
-             const NDArray arg = args(i).array_value ();
+  symbol_record *sym_rec = lookup_by_name (name, 0);
 
-             if (error_state)
-               break;
+  if (sym_rec && sym_rec->is_defined ())
+    retval = sym_rec->which ();
+  else
+    retval = fcn_file_in_path (name);
 
-             for (octave_idx_type j = 0; j < arg.nelem(); j++)
-               {
-                 int line = static_cast<int> (arg.elem (j));
+  return retval;
+}
 
-                 if (error_state)
-                   break;
 
-                 if (nr == nsize)
-                   {
-                     nsize *= 2;
-                     results.resize (nsize);
-                   }
+intmap
+bp_table::add_breakpoint (std::string fname, 
+                         const intmap& line)
+{
+  intmap& linenc = const_cast<intmap&>(line);
+  octave_idx_type len = line.size ();
+  intmap retval;
+  octave_user_function *dbg_fcn = get_user_function (fname);
 
-                 results(nr++) = cmds->set_breakpoint (line);
-               }
+  if (dbg_fcn)
+    {
+      tree_statement_list *cmds = dbg_fcn->body ();
+      for (int i = 0; i < len; i++)
+       {
+         int lineno = linenc[i];
+         retval[i] = cmds->set_breakpoint (lineno);
+         if (retval[i] != 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, 
+                            const intmap& line)
+{
+  intmap& linenc = const_cast<intmap&>(line);
+  octave_idx_type len = line.size ();
+  int retval = 0;
+
+  if (len == 0)
+    {
+      intmap results = remove_all_breakpoints_in_file (fname);
+      retval = results.size ();
+    }
+  else
+    {
+      octave_user_function *dbg_fcn = get_user_function (fname);
+      if (dbg_fcn)
+       {
+         tree_statement_list *cmds = dbg_fcn->body ();
+         for (int i = 0; i < len; i++)
+           {
+             int lineno = linenc[i];
+             cmds->delete_breakpoint (lineno);
            }
+         octave_value_list results = cmds->list_breakpoints ();
+         if (results.length () == 0)
+           bp_map.erase (bp_map.find (fname));
+         retval = results.length ();
        }
+      else
+       error ("remove_breakpoint: unable to find the function requested\n");
+    }
+  return retval;
+}
+
 
-      if (! error_state)
+intmap
+bp_table::remove_all_breakpoints_in_file (std::string fname)
+{
+  octave_value_list bkpts;
+  intmap retval;
+  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;
+         int lineno = static_cast<int> (bkpts (i).int_value ());
+         cmds->delete_breakpoint (lineno);
+         retval[i] = lineno;
        }
+      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 retval;
 }
 
+
+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, intmap> 
+bp_table::get_breakpoint_list (const octave_value_list& fname_list)
+{
+  std::map<std::string, intmap> 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 ();
+         octave_idx_type len = bkpts.length (); 
+         intmap bkpts_vec;
+         for (int i = 0; i < len; i++)
+           bkpts_vec[i] = bkpts (i).double_value ();
+         retval[ it->first ] = bkpts_vec;
+       }
+    }
+  return retval;
+}
+
+static octave_value
+intmap_to_ov (const intmap& line) 
+{
+  intmap& linenc = const_cast<intmap&> (line);
+  NDArray retval (dim_vector (1, line.size()));
+  for (int i = 0; i < line.size(); i++ )
+    retval(i) = linenc[i];
+  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")
+{
+  intmap retval;
+  std::string symbol_name = "";
+  intmap lines;
+  parse_dbfunction_params (args, symbol_name, lines);
+
+  if (!error_state)
+    retval = breakpoints.add_breakpoint (symbol_name, lines);
+
+  return intmap_to_ov(retval);
+}
+
 DEFCMD (dbclear, args, ,
   "-*- texinfo -*-\n\
 @deftypefn {Loadable Function} {} dbclear (func, line, @dots{})\n\
@@ -197,62 +367,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");
-
+  intmap 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 +390,76 @@
 @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, intmap> 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 ();
     }
 
-  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));
+       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, intmap>::iterator it;
+  if (nargout == 1)
+    {
+      // Fill in an array for return
+      int i = 0;
+      Cell namesCell (dim_vector (bp_list.size (), 1));
+      Cell fileCell (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;
+         // Must convert it->second to octave_value rowvector
+         //lineCell (i)  = it->second;
+         lineCell (i) = intmap_to_ov(it->second);
+         fileCell (i)  = do_which (it->first);
+         i++;
        }
-
-      retval = octave_value (vec);
+      retval.assign ("name", namesCell);
+      retval.assign ("file", fileCell);
+      retval.assign ("line", lineCell);
+      return octave_value (retval);
     }
   else
-    error ("dbstatus: unable to find the function you requested\n");
-
-  return retval;
+    {
+      // 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.size (); j++)
+           if (j < it->second.size()-1)
+             octave_stdout << it->second [j] << ", ";
+           else
+             octave_stdout << it->second [j] << "." << std::endl;
+       }
+      return octave_value ();
+    }
 }
 
 DEFCMD (dbwhere, , ,

--- /dev/null   2007-10-29 13:21:35.000000000 -0400
+++ debug.h     2007-10-29 14:24:40.000000000 -0400
@@ -0,0 +1,82 @@
+/*
+
+Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006, 2007 Ben Sapp
+
+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/>.
+
+*/
+
+#if !defined (octave_debug_h)
+#define octave_debug_h 1
+
+#include <map>
+#include "ov.h"
+#include "dRowVector.h"
+
+//class RowVector;
+class octave_value_list;
+class octave_user_function;
+
+typedef std::map<int, int> intmap;
+
+// 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:
+
+  bp_table (void) {};
+
+  ~bp_table (void) {};
+
+  // Add a breakpoint at the nearest executable line.
+  intmap add_breakpoint (std::string fname = "", 
+                               const intmap& lines = intmap());
+
+  // Remove a breakpoint from a line in file.
+  int remove_breakpoint (std::string fname = "", 
+                        const intmap& lines = intmap());
+
+  // Remove all the breakpoints in a specified file.
+  intmap remove_all_breakpoints_in_file (std::string fname);
+  
+  // Remove all the breakpoints registered with octave.
+  void remove_all_breakpoints (void);
+  
+  // Return all breakpoints.  Each element of the map is a vector
+  // containing the breakpoints corresponding to a given function name.
+  std::map <std::string, intmap> 
+  get_breakpoint_list (const octave_value_list& fname_list);
+
+private:
+
+  // Map from function names to function objects for functions
+  // containing at least one breakpoint.
+  std::map<std::string, octave_user_function*> bp_map;
+};
+
+// Table of breakpoints
+extern bp_table breakpoints;
+
+#endif
+
+/*
+;;; Local Variables: ***
+;;; mode: C++ ***
+;;; End: ***
+*/