[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Proposed patch for external debugger access
From: |
John Swensen |
Subject: |
Re: Proposed patch for external debugger access |
Date: |
Mon, 29 Oct 2007 23:01:06 -0400 |
User-agent: |
Thunderbird 2.0.0.6 (Macintosh/20070728) |
John W. Eaton wrote:
On 29-Oct-2007, John Swensen wrote:
| 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.
If it is the same function, then I don't think there is any reason to
duplicate it. We should remove the static qualifier from the function
in help.cc instead.
Included in the patch
| > // 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.
OK.
| +intmap
| +bp_table::add_breakpoint (std::string fname,
| + const intmap& line)
| +{
| + intmap& linenc = const_cast<intmap&>(line);
Why are you casting away const here? It seems the only use of linenc
is on the RHS of an assignment:
| + int lineno = linenc[i];
so it seems that it should be OK to declare it const here. Oh, intmap
is std::map<int,int> and there is no const operator [] method (because
the std::map operator [] always inserts an entry for a missing key) so
I think you need to use something like
std::map<int, int>::const_iterator p = line.seek (i);
if (p != line.end ())
{
int lineno = p->second;
...
}
else
// Handle case of missing index...
BTW, I think the behavior would be unpredictible if you cast away
const and the operator [] method actually did insert something.
Fixed this.
| +intmap
| +bp_table::remove_all_breakpoints_in_file (std::string fname)
This should be "const std::string& fname".
Fixed several of these.
| + Cell namesCell (dim_vector (bp_list.size (), 1));
| + Cell fileCell (dim_vector (bp_list.size (), 1));
| + Cell lineCell (dim_vector (bp_list.size (), 1));
I usually find that it is better to avoid including the data type in
the variable name.
Done.
| +// 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:
|
| [...]
|
| +};
| +
| +// Table of breakpoints
| +extern bp_table breakpoints;
Is there only one table of breakpoints? If so, this should probably
be a singleton class.
Made it a singleton class.
jwe
I fixed up these suggestions also. The patches are attached.
John Swensen
Index: debug.cc
===================================================================
RCS file: /cvs/octave/src/debug.cc,v
retrieving revision 1.26
diff -r1.26 debug.cc
22d21
<
29a29,30
> #include <set>
>
32a34
> #include "help.h"
42a45,46
> #include "ov-list.h"
> #include "ov-struct.h"
50a55,59
> #include "debug.h"
>
> // Initialize the singleton object
> bp_table *bp_table::instance = NULL;
>
54d62
<
56c64
< get_user_function (std::string fname = "")
---
> get_user_function (const std::string& fname = "")
74d81
<
86,102c93,96
<
< DEFCMD (dbstop, args, ,
< "-*- texinfo -*-\n\
< @deftypefn {Loadable Function} {rline =} dbstop (func, line, @dots{})\n\
< Set a breakpoint in a function\n\
< @table @code\n\
< @item 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\
< @item 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\
< @end table\n\
< \n\
< The rline returned is the real line that the breakpoint was set at.\n\
< @seealso{dbclear, dbstatus, dbnext}\n\
< @end deftypefn")
---
> static void
> parse_dbfunction_params (const octave_value_list& args,
> std::string& symbol_name,
> intmap& lines)
104c98
< octave_value retval;
---
> octave_idx_type len = 0;
107c101,102
< std::string symbol_name = "";
---
> int list_idx = 0;
> symbol_name = std::string ();
109c104,105
< if (nargin != 1 && args(0).is_string())
---
> // If we are already in a debugging function
> if (octave_call_stack::caller_user_function () != NULL)
111c107,113
< symbol_name = args(0).string_value ();
---
> idx = 0;
> }
> else
> {
> symbol_name = args (0).string_value ();
> if (error_state)
> return;
115,117c117
< octave_user_function *dbg_fcn = get_user_function (symbol_name);
<
< if (dbg_fcn)
---
> for (int i = idx; i < nargin; i++ )
119,123c119,123
< 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 ();
> }
125c125,135
< 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
127c137,142
< 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++)
129,130c144
< int line = atoi (args(i).string_value ().c_str ());
<
---
> int line = static_cast<int> (arg.elem (j));
132a147,154
> lines[list_idx++] = line;
> }
>
> if (error_state)
> break;
> }
> }
> }
134,138c156,161
< if (nr == nsize)
< {
< nsize *= 2;
< results.resize (nsize);
< }
---
> intmap
> bp_table::add_breakpoint (const std::string& fname,
> const intmap& line)
> {
> if (!instance_ok ())
> return intmap();
140,142c163,173
< results(nr++) = cmds->set_breakpoint (line);
< }
< else
---
> octave_idx_type len = line.size ();
> intmap 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 < len; i++)
> {
> intmap::const_iterator p = line.find (i);
> if (p != line.end ())
144c175,183
< const NDArray arg = args(i).array_value ();
---
> int lineno = p->second;
> retval[i] = cmds->set_breakpoint (lineno);
> if (retval[i] != 0)
> instance->bp_map[fname] = dbg_fcn;
> }
> }
> }
> else
> error ("add_breakpoint: unable to find the function requested\n");
146,147c185,186
< if (error_state)
< break;
---
> return retval;
> }
149,151d187
< for (octave_idx_type j = 0; j < arg.nelem(); j++)
< {
< int line = static_cast<int> (arg.elem (j));
153,154c189,194
< if (error_state)
< break;
---
> int
> bp_table::remove_breakpoint (const std::string& fname,
> const intmap& line)
> {
> if (!instance_ok ())
> return 0;
156,160c196,197
< if (nr == nsize)
< {
< nsize *= 2;
< results.resize (nsize);
< }
---
> octave_idx_type len = line.size ();
> int retval = 0;
162c199,216
< results(nr++) = cmds->set_breakpoint (line);
---
> 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++)
> {
> intmap::const_iterator p = line.find (i);
> if (p != line.end ())
> {
> int lineno = p->second;
> cmds->delete_breakpoint (lineno);
164,166d217
<
< if (error_state)
< break;
167a219,222
> octave_value_list results = cmds->list_breakpoints ();
> if (results.length () == 0)
> instance->bp_map.erase (instance->bp_map.find (fname));
> retval = results.length ();
168a224,235
> else
> error ("remove_breakpoint: unable to find the function requested\n");
> }
> return retval;
> }
>
>
> intmap
> bp_table::remove_all_breakpoints_in_file (const std::string& fname)
> {
> if (!instance_ok ())
> return intmap();
170c237,245
< if (! error_state)
---
> 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++)
172,173c247,249
< results.resize (nr);
< retval = results;
---
> int lineno = static_cast<int> (bkpts (i).int_value ());
> cmds->delete_breakpoint (lineno);
> retval[i] = lineno;
174a251
> instance->bp_map.erase (instance->bp_map.find (fname));
177c254,316
< 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)
> {
> if (!instance_ok ())
> return;
>
> std::map< std::string, octave_user_function* >::iterator it;
> for (it = instance->bp_map.begin (); it != instance->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;
>
> if (!instance_ok ())
> return 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 = instance->bp_map.begin (); it != instance->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;
> }
178a318,332
> static octave_value
> intmap_to_ov (const intmap& line)
> {
> int idx = 0;
> NDArray retval (dim_vector (1, line.size()));
> for (int i = 0; i < line.size(); i++ )
> {
> intmap::const_iterator p = line.find (i);
> if (p != line.end ())
> {
> int lineno = p->second;
> retval (idx++) = lineno;
> }
> }
> retval.resize (dim_vector (1, idx));
181a336,363
> DEFCMD (dbstop, args, ,
> "-*- texinfo -*-\n\
> @deftypefn {Loadable Function} {rline =} dbstop (func, line, @dots{})\n\
> Set a breakpoint in a function\n\
> @table @code\n\
> @item 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\
> @item 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\
> @end table\n\
> \n\
> The rline returned is the real line that the breakpoint was set at.\n\
> @seealso{dbclear, dbstatus, dbnext}\n\
> @end deftypefn")
> {
> intmap retval;
> std::string symbol_name = "";
> intmap lines;
> parse_dbfunction_params (args, symbol_name, lines);
>
> if (!error_state)
> retval = bp_table::add_breakpoint (symbol_name, lines);
>
> return intmap_to_ov(retval);
> }
>
200,201d381
< int nargin = args.length ();
< int idx = 0;
203,250c383,387
<
< 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)
> bp_table::remove_breakpoint (symbol_name, lines);
255c392
< DEFCMD (dbstatus, args, ,
---
> DEFCMD (dbstatus, args, nargout,
268,269c405
< octave_value retval;
<
---
> Octave_map retval;
270a407,409
> octave_value_list fcn_list;
> std::map< std::string, intmap> bp_list;
> std::string symbol_name = "";
275c414
< return retval;
---
> return octave_value ();
278,279d416
< std::string symbol_name = "";
<
283c420,424
< symbol_name = args(0).string_value ();
---
> {
> symbol_name = args (0).string_value ();
> fcn_list (0) = symbol_name;
> bp_list = bp_table::get_breakpoint_list (fcn_list);
> }
285c426
< gripe_wrong_type_arg ("dbstatus", args(0));
---
> gripe_wrong_type_arg ("dbstatus", args (0));
287,290c428
<
< octave_user_function *dbg_fcn = get_user_function (symbol_name);
<
< if (dbg_fcn)
---
> else
292,298c430,447
< 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 = bp_table::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 names (dim_vector (bp_list.size (), 1));
> Cell file (dim_vector (bp_list.size (), 1));
> Cell line (dim_vector (bp_list.size (), 1));
> for (it = bp_list.begin (); it != bp_list.end (); it++)
300,303c449,452
< vec(i) = lst(i).double_value ();
<
< if (error_state)
< panic_impossible ();
---
> names (i) = it->first;
> line (i) = intmap_to_ov(it->second);
> file (i) = do_which (it->first);
> i++;
305,306c454,457
<
< retval = octave_value (vec);
---
> retval.assign ("name", names);
> retval.assign ("file", file);
> retval.assign ("line", line);
> return octave_value (retval);
309,311c460,472
< 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 ();
> }
Index: help.h
===================================================================
RCS file: /cvs/octave/src/help.h,v
retrieving revision 1.27
diff -r1.27 help.h
51a52,53
> extern std::string do_which (const std::string& name);
>
Index: help.cc
===================================================================
RCS file: /cvs/octave/src/help.cc,v
retrieving revision 1.175
diff -r1.175 help.cc
1298c1298
< static std::string
---
> std::string
--- /dev/null 2007-10-29 22:54:34.000000000 -0400
+++ debug.h 2007-10-29 22:57:32.000000000 -0400
@@ -0,0 +1,101 @@
+/*
+
+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 singleton 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
+{
+private:
+
+ bp_table (void) {};
+
+ ~bp_table (void) {};
+
+public:
+
+ static bool instance_ok (void)
+ {
+ bool retval = true;
+
+ if (! instance)
+ instance = new bp_table ();
+
+ if (! instance)
+ {
+ ::error ("unable to create breakpoint table!");
+ retval = false;
+ }
+
+ return retval;
+ }
+
+ // Add a breakpoint at the nearest executable line.
+ static intmap add_breakpoint (const std::string& fname = "",
+ const intmap& lines = intmap());
+
+ // Remove a breakpoint from a line in file.
+ static int remove_breakpoint (const std::string& fname = "",
+ const intmap& lines = intmap());
+
+ // Remove all the breakpoints in a specified file.
+ static intmap remove_all_breakpoints_in_file (const std::string& fname);
+
+ // Remove all the breakpoints registered with octave.
+ static 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.
+ static 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;
+
+ // Singleton instance
+ static bp_table *instance;
+};
+
+
+#endif
+
+/*
+;;; Local Variables: ***
+;;; mode: C++ ***
+;;; End: ***
+*/
- Proposed patch for external debugger access, John Swensen, 2007/10/23
- Proposed patch for external debugger access, John W. Eaton, 2007/10/24
- Re: Proposed patch for external debugger access, John Swensen, 2007/10/24
- Re: Proposed patch for external debugger access, John Swensen, 2007/10/24
- Re: Proposed patch for external debugger access, John W. Eaton, 2007/10/24
- Re: Proposed patch for external debugger access, John Swensen, 2007/10/29
- Re: Proposed patch for external debugger access, John W. Eaton, 2007/10/29
- Re: Proposed patch for external debugger access,
John Swensen <=
- Re: Proposed patch for external debugger access, John W. Eaton, 2007/10/30
- Re: Proposed patch for external debugger access, John Swensen, 2007/10/30
- Re: Proposed patch for external debugger access, John W. Eaton, 2007/10/30
- Re: Proposed patch for external debugger access, John Swensen, 2007/10/31
- Re: Proposed patch for external debugger access, John W. Eaton, 2007/10/31
- Re: Proposed patch for external debugger access, John Swensen, 2007/10/31
- Re: Proposed patch for external debugger access, John W. Eaton, 2007/10/31