Mercurial > hgweb > octave
changeset 32279:fbd7e00d3e6e
Add support for classdef breakpoints (bug #46451).
* libinterp/octave-value/cdef-class.cc
(cdef_class::cdef_class_rep::get_method (int line) const): Add new member
function.
* libinterp/parse-tree/bp-table.cc (user_code_provider): Add new class to
enable setting breakpoints and using the dbstatus and dbclear commands within
classdef classes.
* libinterp/octave-value/cdef-class.h
(octave_value get_method (int line) const): Add new function.
* test/classdef-debug/breakpoints/test_classdef_breakpoints.m: Add new script
for testing breakpoints functionality.
* test/classdef-debug/breakpoints/classdef_breakpoints.m: Add new file
containing a simple classdef class for use in testing the new functionality.
* test/classdef-debug/module.mk: Add new file.
* test/Makefile.am: Include new module.
| author | paulo <paulo.fernando.silva@gmail.com> |
|---|---|
| date | Sun, 11 Jun 2023 11:08:05 +0900 |
| parents | 3de9f020ce41 |
| children | 6f39259f20be |
| files | libinterp/octave-value/cdef-class.cc libinterp/octave-value/cdef-class.h libinterp/parse-tree/bp-table.cc test/Makefile.am test/classdef-debug/classdef_breakpoints.m test/classdef-debug/module.mk test/classdef-debug/test_classdef_breakpoints.tst |
| diffstat | 7 files changed, 312 insertions(+), 62 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/octave-value/cdef-class.cc +++ b/libinterp/octave-value/cdef-class.cc @@ -708,6 +708,39 @@ return p->second.get_function (); } +octave_value +cdef_class::cdef_class_rep::get_method (int line) const +{ + octave_value closest_match; + int closest_match_end_line = std::numeric_limits<int>::max (); + // Since we have a dynamic cast, performance could be an issue if this is + // called from a critical path. If performance is an issue, we can cache + // an ordered version of the method map + for (auto i = m_method_map.cbegin (); i != m_method_map.cend (); ++i) + { + const octave_value& fcn = i->second.get_function (); + octave_user_code *user_code = fcn.user_code_value (); + + if (user_code == nullptr) + continue; + + octave_user_function* pfcn + = dynamic_cast<octave_user_function*> (user_code); + + if (pfcn == nullptr) + continue; + + const int e = pfcn->ending_line (); + if (line <= e && e <= closest_match_end_line && pfcn->is_defined () + && pfcn->is_user_code ()) + { + closest_match = fcn; + closest_match_end_line = e; + } + } + + return closest_match; +} octave_value cdef_class::cdef_class_rep::construct (const octave_value_list& args)
--- a/libinterp/octave-value/cdef-class.h +++ b/libinterp/octave-value/cdef-class.h @@ -124,6 +124,8 @@ OCTINTERP_API octave_value get_method (const std::string& name) const; + OCTINTERP_API octave_value get_method (int line) const; + OCTINTERP_API octave_value construct (const octave_value_list& args); OCTINTERP_API cdef_object @@ -357,6 +359,11 @@ return get_rep ()->get_method (nm); } + octave_value get_method (int ln) const + { + return get_rep ()->get_method (ln); + } + OCTINTERP_API octave_value get_method_function (const std::string& nm); octave_value get_constructor_function ()
--- a/libinterp/parse-tree/bp-table.cc +++ b/libinterp/parse-tree/bp-table.cc @@ -33,11 +33,13 @@ #include <map> #include <set> #include <string> +#include <vector> #include "file-ops.h" #include "oct-env.h" #include "bp-table.h" +#include "cdef-utils.h" #include "defun-int.h" #include "error.h" #include "event-manager.h" @@ -607,8 +609,8 @@ // If END_LINE != 0, *END_LINE is set to last line of the returned function. static octave_user_code * find_fcn_by_line (octave_user_code *main_fcn, - int lineno, - int *end_line = nullptr) + int lineno, + int *end_line = nullptr) { octave_user_code *retval = nullptr; octave_user_code *next_fcn = nullptr; // 1st function starting after lineno @@ -682,6 +684,102 @@ return result.empty () ? 0 : *(result.begin ()); } +// A class that encapsulates the "get_user_code" operation. +// This operation changes depending on the symbol being a function or classdef. +// Octave used to assume everything was a function, one per file. +// Classdefs can have several functions in a single file. +class user_code_provider +{ +public: + user_code_provider (const std::string& fname, octave_user_code* pfcn) + : m_fcn (nullptr), m_is_valid (false) + { + if (pfcn) + { + // Already have the usercode, no need to do anything. + m_fcn = pfcn; + m_is_valid = true; + } + else + { + // Independently of cname being empty, if we get here, try to get + // a classdef: + // Is it a classdef file plus line numbers? + m_cls = lookup_class (fname, false, false); + m_is_valid = m_cls.ok () && (m_cls.get_name () == fname); + if (m_is_valid) + populate_function_cache (); + else + error ("add_breakpoints_in_function: unable to find function '%s'\n", + fname.c_str ()); + } + } + + octave_user_code * operator () (int line = 1) + { + if (! is_valid ()) + return nullptr; + + if (is_function ()) + return find_fcn_by_line (m_fcn, line); + + return m_cls.get_method (line).user_code_value (true); + } + + bool is_function () const { return m_fcn != nullptr; } + + bool is_valid () const { return m_is_valid; } + + size_t number_of_functions () const + { + if (! is_valid ()) + return 0; + + if (is_function ()) + return 1; + + return m_methods_cache.size (); + } + + octave_user_code * get_function (const size_t i) const + { + if (! is_valid ()) + return nullptr; + + if (is_function ()) + return m_fcn; + + if (i < m_methods_cache.size ()) + return m_methods_cache[i]; + + return nullptr; + } + +private: + void populate_function_cache () + { + if (m_methods_cache.empty ()) + { + // Not the most effecient, but reuses code: + const std::map<std::string, cdef_method>& map + = m_cls.get_method_map (false, true); + for (auto iter = map.cbegin (); iter != map.cend (); ++iter) + { + octave_user_code *fcn + = m_cls.get_method (iter->first).user_code_value (true); + if (fcn != nullptr) + m_methods_cache.push_back (fcn); + } + } + } + +private: + std::vector<octave_user_code *> m_methods_cache; + cdef_class m_cls; + octave_user_code *m_fcn; + bool m_is_valid; +}; + // Given file name fname, find the subfunction at line and create // a breakpoint there. Put the system into debug_mode. bp_table::bp_lines @@ -690,11 +788,8 @@ const bp_table::bp_lines& lines, const std::string& condition) { - octave_user_code *main_fcn = m_evaluator.get_user_code (fname, class_name); - - if (! main_fcn) - error ("add_breakpoints_in_function: unable to find function '%s'\n", - fname.c_str ()); + user_code_provider + user_code (fname, m_evaluator.get_user_code (fname, class_name)); condition_valid (condition); // Throw error if condition not valid. @@ -702,7 +797,7 @@ for (const auto& lineno : lines) { - octave_user_code *dbg_fcn = find_fcn_by_line (main_fcn, lineno); + octave_user_code *dbg_fcn = user_code (lineno); // We've found the right (sub)function. Now insert the breakpoint. bp_lines line_info; @@ -814,7 +909,7 @@ int bp_table::remove_breakpoints_from_function (const std::string& fname, - const bp_table::bp_lines& lines) + const bp_table::bp_lines& lines) { int retval = 0; @@ -826,32 +921,68 @@ else { octave_user_code *dbg_fcn = m_evaluator.get_user_code (fname); + user_code_provider user_code (fname, dbg_fcn); - if (! dbg_fcn) + if (user_code.is_valid ()) + { + // Remove all breakpoints from all functions + for (const auto line : lines) + { + octave_user_code *fcn = user_code (line); + std::string file = fcn->fcn_file_name (); + + tree_statement_list *cmds = fcn->body (); + if (cmds) + { + octave_value_list results = cmds->list_breakpoints (); + + if (results.length () > 0) + { + interpreter& interp = m_evaluator.get_interpreter (); + event_manager& evmgr = interp.get_event_manager (); + + cmds->delete_breakpoint (line); + + if (! file.empty ()) + evmgr.update_breakpoint (false, file, line); + } + } + } + + // Remove all breakpoints from all subfunctions + if (dbg_fcn != nullptr) + { + // Search subfunctions in the order they appear in the file. + const std::list<std::string> subfcn_names + = dbg_fcn->subfunction_names (); + + std::map<std::string, octave_value> subfcns + = dbg_fcn->subfunctions (); + + for (const auto& subf_nm : subfcn_names) + { + const auto q = subfcns.find (subf_nm); + + if (q != subfcns.end ()) + { + octave_user_code *dbg_subfcn + = q->second.user_code_value (); + + retval += remove_breakpoint_1 (dbg_subfcn, fname, lines); + } + } + } + // Remove file from breakpoint set if no breakpoints remain + octave_value_list fname_list = {fname}; + const bool no_breakpoints + = get_breakpoint_list (fname_list).empty (); + auto iter = m_bp_set.find (fname); + if (no_breakpoints && iter != m_bp_set.end ()) + m_bp_set.erase (iter); + } + else error ("remove_breakpoints_from_function: unable to find function %s\n", fname.c_str ()); - - retval = remove_breakpoint_1 (dbg_fcn, fname, lines); - - // Search subfunctions in the order they appear in the file. - - const std::list<std::string> subfcn_names - = dbg_fcn->subfunction_names (); - - std::map<std::string, octave_value> subfcns - = dbg_fcn->subfunctions (); - - for (const auto& subf_nm : subfcn_names) - { - const auto q = subfcns.find (subf_nm); - - if (q != subfcns.end ()) - { - octave_user_code *dbg_subfcn = q->second.user_code_value (); - - retval += remove_breakpoint_1 (dbg_subfcn, fname, lines); - } - } } m_evaluator.reset_debug_state (); @@ -863,30 +994,34 @@ bp_table::bp_lines bp_table::remove_all_breakpoints_from_function (const std::string& fname, - bool silent) + bool silent) { bp_lines retval; - octave_user_code *dbg_fcn = m_evaluator.get_user_code (fname); + octave_user_code *fcn = m_evaluator.get_user_code (fname); + user_code_provider user_code (fname, fcn); - if (dbg_fcn) + if (user_code.is_valid ()) { - std::string file = dbg_fcn->fcn_file_name (); + for (size_t i = 0; i != user_code.number_of_functions (); ++i) + { + octave_user_code *dbg_fcn = user_code.get_function (i); + std::string file = dbg_fcn->fcn_file_name (); - tree_statement_list *cmds = dbg_fcn->body (); + tree_statement_list *cmds = dbg_fcn->body (); - if (cmds) - { - interpreter& interp = m_evaluator.get_interpreter (); + if (cmds) + { + interpreter& interp = m_evaluator.get_interpreter (); - event_manager& evmgr = interp.get_event_manager (); + event_manager& evmgr = interp.get_event_manager (); - retval = cmds->remove_all_breakpoints (evmgr, file); - - auto it = m_bp_set.find (fname); - if (it != m_bp_set.end ()) - m_bp_set.erase (it); + retval = cmds->remove_all_breakpoints (evmgr, file); + } } + auto it = m_bp_set.find (fname); + if (it != m_bp_set.end ()) + m_bp_set.erase (it); } else if (! silent) error ("remove_all_breakpoints_from_function: " @@ -928,7 +1063,7 @@ bp_table::bp_lines bp_table::remove_all_breakpoints_from_file (const std::string& file, - bool silent) + bool silent) { // Duplicates what the GUI was doing previously, but this action // should not be specific to the GUI. @@ -986,22 +1121,39 @@ || find_bkpt_list (fname_list, bp_fname) != "") { octave_user_code *dbg_fcn = m_evaluator.get_user_code (bp_fname); + user_code_provider user_code (bp_fname, dbg_fcn); + // Gather breakpoints from all functions in the file + if (user_code.is_valid ()) + { + std::list<bp_type> all_bkpts; + std::list<bp_type>::iterator it (all_bkpts.begin ()); + for (size_t i = 0; i != user_code.number_of_functions (); ++i) + { + octave_user_code *fcn = user_code.get_function (i); + if (fcn) + { + tree_statement_list *cmds = fcn->body (); + // FIXME: move the operation on cmds to the + // tree_statement_list class? + if (cmds) + { + const std::list<bp_type>& bp + = cmds->breakpoints_and_conds (); + if(!bp.empty()) + it = all_bkpts.insert (it, bp.cbegin (), + bp.cend ()); + } + } - if (dbg_fcn) + } + if (! all_bkpts.empty ()) + retval[bp_fname] = all_bkpts; + } + + // look for breakpoints in subfunctions + // Assuming classdefs can't have subfunctions + if (dbg_fcn != nullptr) { - tree_statement_list *cmds = dbg_fcn->body (); - - // FIXME: move the operation on cmds to the - // tree_statement_list class? - if (cmds) - { - std::list<bp_type> bkpts = cmds->breakpoints_and_conds (); - - if (! bkpts.empty ()) - retval[bp_fname] = bkpts; - } - - // look for breakpoints in subfunctions const std::list<std::string> subf_nm = dbg_fcn->subfunction_names (); @@ -1017,7 +1169,7 @@ octave_user_code *dbg_subfcn = q->second.user_code_value (); - cmds = dbg_subfcn->body (); + tree_statement_list *cmds = dbg_subfcn->body (); if (cmds) { std::list<bp_type> bkpts
--- a/test/Makefile.am +++ b/test/Makefile.am @@ -109,6 +109,7 @@ include bug-61191/module.mk include class-concat/module.mk include classdef/module.mk +include classdef-debug/module.mk include classdef-multiple-inheritance/module.mk include classes/module.mk include colon-op/module.mk
new file mode 100644 --- /dev/null +++ b/test/classdef-debug/classdef_breakpoints.m @@ -0,0 +1,21 @@ +classdef classdef_breakpoints + properties + m_prop = []; + endproperties + methods + function this = classdef_breakpoints (prop) + this.m_prop = prop; + endfunction + function this = foo (this, prop) + this.m_prop = prop; + endfunction + function this = bar (this) + printf ("prop: %f\n", this.m_prop); + endfunction + endmethods +endclassdef + +function my_subfunction (n) + ## my_subfunction hello + printf ("subfunction: %f\n", n); +endfunction
new file mode 100644 --- /dev/null +++ b/test/classdef-debug/module.mk @@ -0,0 +1,5 @@ +classdef_breakpoints_TEST_FILES = \ + %reldir%/classdef_breakpoints.m \ + %reldir%/test_classdef_breakpoints.tst + +TEST_FILES += $(classdef_breakpoints_TEST_FILES)
new file mode 100644 --- /dev/null +++ b/test/classdef-debug/test_classdef_breakpoints.tst @@ -0,0 +1,31 @@ +## Test script for setting breakpoints in classdefs using line numbers. + +%!function assert_dbstatus (bp0) +%! dbs = dbstatus (); +%! bp = [dbs(:).line]; +%! assert (length (bp) == length (bp0), +%! 'Number of breakpoints set must match'); +%! if (length (bp) != 0) +%! assert (sort (bp) == bp0, 'Breakpoint line numbers must be equal'); +%! endif +%!endfunction + +## Add breakpoints in different member functions using line numbers. +%!test <*46451> +%! dbstop classdef_breakpoints 13 7 10; +%! assert_dbstatus ([7, 10, 13]); + +## Remove one breakpoint and confirm the others remain. +%!test <*46451> +%! dbclear classdef_breakpoints 10; +%! assert_dbstatus ([7, 13]); + +## Add breakpoint in local function. +%!test <46451> +%! dbstop classdef_breakpoints 19; +%! assert_dbstatus ([7, 13, 20]); + +## Clear all breakpoints, none should be left. +%!test <*46451> +%! dbclear classdef_breakpoints; +%! assert_dbstatus ([]);
