changeset 29457:313b8b897733

revamp handling of breakpoint line positions Use set<int> instead of map<int,int> since we don't assign number IDs to breakpoints, we just use line numbers. Allow breakpoint functions to accept individual line number arguments. Simplify iteration over sets of breakpoint lines. * debug.cc (bp_lines_to_ov): Rename from intmap_to_ov. * bp-table.h, bp-table.cc (bp_table::bp_lines): New typedef to replace bp_table::intmap. Change all uses. (bp_table::add_breakpoint, bp_table::remove_breakpoint): New overloads that accept single lines as integers instead of requiring a set of lines.
author John W. Eaton <jwe@octave.org>
date Wed, 03 Mar 2021 23:57:31 -0500
parents ebc3f80673f0
children c0f86150aa6c
files libgui/src/m-editor/file-editor-tab.cc libinterp/corefcn/debug.cc libinterp/parse-tree/bp-table.cc libinterp/parse-tree/bp-table.h libinterp/parse-tree/pt-stmt.cc libinterp/parse-tree/pt-stmt.h
diffstat 6 files changed, 123 insertions(+), 129 deletions(-) [+]
line wrap: on
line diff
--- a/libgui/src/m-editor/file-editor-tab.cc	Sun Mar 21 10:53:11 2021 +0100
+++ b/libgui/src/m-editor/file-editor-tab.cc	Wed Mar 03 23:57:31 2021 -0500
@@ -1175,16 +1175,13 @@
 
          load_path& lp = interp.get_load_path ();
 
-         bp_table::intmap line_info;
-         line_info[0] = info.line;
-
          if (lp.contains_file_in_dir (info.file, info.dir))
            {
              tree_evaluator& tw = interp.get_evaluator ();
 
              bp_table& bptab = tw.get_bp_table ();
 
-             bptab.remove_breakpoint (info.function_name, line_info);
+             bptab.remove_breakpoint (info.function_name, info.line);
            }
        });
   }
@@ -1375,27 +1372,17 @@
 
          load_path& lp = interp.get_load_path ();
 
-         bp_table::intmap line_info;
-         line_info[0] = info.line;
-
          if (lp.contains_file_in_dir (info.file, info.dir))
            {
              tree_evaluator& tw = interp.get_evaluator ();
 
              bp_table& bptab = tw.get_bp_table ();
 
-             bp_table::intmap bpmap
-               = bptab.add_breakpoint (info.function_name, "", line_info,
-                                       info.condition);
-
-             if (! bpmap.empty ())
-               {
-                 bp_table::intmap::iterator bp_it = bpmap.begin ();
-
-                 int remove_line = bp_it->second;
-
-                 emit maybe_remove_next (remove_line);
-               }
+             int lineno = bptab.add_breakpoint (info.function_name, "",
+                                                info.line, info.condition);
+
+             if (lineno)
+               emit maybe_remove_next (lineno);
            }
        });
   }
--- a/libinterp/corefcn/debug.cc	Sun Mar 21 10:53:11 2021 +0100
+++ b/libinterp/corefcn/debug.cc	Wed Mar 03 23:57:31 2021 -0500
@@ -60,22 +60,14 @@
 #include "variables.h"
 
 static octave_value
-intmap_to_ov (const octave::bp_table::intmap& line)
+bp_lines_to_ov (const octave::bp_table::bp_lines& lines)
 {
   int idx = 0;
 
-  NDArray retval (dim_vector (1, line.size ()));
-
-  for (size_t i = 0; i < line.size (); i++)
-    {
-      octave::bp_table::const_intmap_iterator p = line.find (i);
+  NDArray retval (dim_vector (1, lines.size ()));
 
-      if (p != line.end ())
-        {
-          int lineno = p->second;
-          retval(idx++) = lineno;
-        }
-    }
+  for (const auto& lineno : lines)
+    retval(idx++) = lineno;
 
   retval.resize (dim_vector (1, idx));
 
@@ -173,10 +165,10 @@
 @seealso{dbclear, dbstatus, dbstep, debug_on_error, debug_on_warning, debug_on_interrupt}
 @end deftypefn */)
 {
-  octave::bp_table::intmap retmap;
+  octave::bp_table::bp_lines retmap;
   std::string symbol_name = "";  // stays empty for "dbstop if error" etc
   std::string class_name = "";
-  octave::bp_table::intmap lines;
+  octave::bp_table::bp_lines lines;
   std::string condition = "";
   octave_value retval;
 
@@ -191,13 +183,13 @@
                                      class_name, lines, condition);
 
       if (lines.size () == 0)
-        lines[0] = 1;
+        lines.insert (1);
 
       if (symbol_name != "")
         {
           retmap = bptab.add_breakpoint (symbol_name, class_name,
                                          lines, condition);
-          retval = intmap_to_ov (retmap);
+          retval = bp_lines_to_ov (retmap);
         }
     }
   else if (args.length () != 1)
@@ -242,7 +234,7 @@
           std::string unconditional = "";
           for (octave_idx_type i = 0; i < line.numel (); i++)
             {
-              lines [0] = line(i).double_value ();
+              lines.insert (line(i).int_value ());
               bptab.add_breakpoint (name(i).string_value (), "", lines,
                                     (use_cond
                                      ? cond(i).string_value ()
@@ -304,7 +296,7 @@
 {
   std::string symbol_name = "";  // stays empty for "dbclear if error" etc
   std::string class_name = "";
-  octave::bp_table::intmap lines;
+  octave::bp_table::bp_lines lines;
   std::string dummy;             // "if" condition -- only used for dbstop
 
   int nargin = args.length ();
@@ -313,7 +305,8 @@
 
   octave::bp_table& bptab = tw.get_bp_table ();
 
-  bptab.parse_dbfunction_params ("dbclear", args, symbol_name, class_name, lines, dummy);
+  bptab.parse_dbfunction_params ("dbclear", args, symbol_name, class_name,
+                                 lines, dummy);
 
   if (nargin == 1 && symbol_name == "all")
     {
--- a/libinterp/parse-tree/bp-table.cc	Sun Mar 21 10:53:11 2021 +0100
+++ b/libinterp/parse-tree/bp-table.cc	Wed Mar 03 23:57:31 2021 -0500
@@ -174,9 +174,9 @@
 
   bool bp_table::add_breakpoint_1 (octave_user_code *fcn,
                                    const std::string& fname,
-                                   const bp_table::intmap& line,
+                                   const bp_table::bp_lines& line,
                                    const std::string& condition,
-                                   bp_table::intmap& retval)
+                                   bp_table::bp_lines& retval)
   {
     bool found = false;
 
@@ -192,9 +192,9 @@
 
         retval = cmds->add_breakpoint (evmgr, file, line, condition);
 
-        for (auto& idx_line_p : retval)
+        for (auto& lineno : retval)
           {
-            if (idx_line_p.second != 0)
+            if (lineno != 0)
               {
                 // Normalize to store only the file name.
                 // Otherwise, there can be an entry for both
@@ -283,14 +283,13 @@
                                           const octave_value_list& args,
                                           std::string& func_name,
                                           std::string& class_name,
-                                          bp_table::intmap& lines,
+                                          bp_table::bp_lines& lines,
                                           std::string& cond)
   {
     int nargin = args.length ();
-    int list_idx = 0;
     func_name = "";
     class_name = "";
-    lines = bp_table::intmap ();
+    lines = bp_table::bp_lines ();
 
     if (nargin == 0 || ! args(0).is_string ())
       print_usage (who);
@@ -397,7 +396,7 @@
                     int line = atoi (args(pos).string_value ().c_str ());
 
                     if (line > 0)
-                      lines[list_idx++] = line;
+                      lines.insert (line);
                     else
                       break;        // may be "if" or a method name
                   }
@@ -406,7 +405,7 @@
                     const NDArray arg = args(pos).array_value ();
 
                     for (octave_idx_type j = 0; j < arg.numel (); j++)
-                      lines[list_idx++] = static_cast<int> (arg.elem (j));
+                      lines.insert (static_cast<int> (arg.elem (j)));
                   }
                 else
                   error ("%s: Invalid argument type %s",
@@ -612,10 +611,24 @@
 
   // Given file name fname, find the subfunction at line and create
   // a breakpoint there.  Put the system into debug_mode.
-  bp_table::intmap bp_table::add_breakpoint (const std::string& fname,
-                                             const std::string& class_name,
-                                             const bp_table::intmap& line,
-                                             const std::string& condition)
+  int bp_table::add_breakpoint (const std::string& fname,
+                                const std::string& class_name,
+                                int line, const std::string& condition)
+  {
+    bp_lines line_info;
+    line_info.insert (line);
+
+    bp_lines result = add_breakpoint (fname, class_name, line_info, condition);
+
+    return result.empty () ? 0 : *(result.begin ());
+  }
+
+  // Given file name fname, find the subfunction at line and create
+  // a breakpoint there.  Put the system into debug_mode.
+  bp_table::bp_lines bp_table::add_breakpoint (const std::string& fname,
+                                               const std::string& class_name,
+                                               const bp_table::bp_lines& lines,
+                                               const std::string& condition)
   {
     octave_user_code *main_fcn = m_evaluator.get_user_code (fname, class_name);
 
@@ -624,27 +637,27 @@
 
     condition_valid (condition);  // Throw error if condition not valid.
 
-    intmap retval;
+    bp_lines retval;
 
-    octave_idx_type len = line.size ();
+    for (const auto& lineno : lines)
+      {
+        octave_user_code *dbg_fcn = find_fcn_by_line (main_fcn, lineno);
 
-    for (int i = 0; i < len; i++)
-      {
-        const_intmap_iterator m = line.find (i);
+        // We've found the right (sub)function.  Now insert the breakpoint.
+        bp_lines line_info;
+        line_info.insert (lineno);
 
-        if (m != line.end ())
+        bp_lines ret_one;
+        if (dbg_fcn && add_breakpoint_1 (dbg_fcn, fname, line_info,
+                                         condition, ret_one))
           {
-            int lineno = m->second;
-
-            octave_user_code *dbg_fcn = find_fcn_by_line (main_fcn, lineno);
+            if (! ret_one.empty ())
+              {
+                int line = *(ret_one.begin ());
 
-            // We've found the right (sub)function.  Now insert the breakpoint.
-            // We insert all breakpoints.
-            // If multiple are in the same function, we insert multiple times.
-            intmap ret_one;
-            if (dbg_fcn
-                && add_breakpoint_1 (dbg_fcn, fname, line, condition, ret_one))
-              retval.insert (std::pair<int,int> (i, ret_one.find (i)->second));
+                if (line)
+                  retval.insert (line);
+              }
           }
       }
 
@@ -655,7 +668,7 @@
 
   int bp_table::remove_breakpoint_1 (octave_user_code *fcn,
                                      const std::string& fname,
-                                     const bp_table::intmap& line)
+                                     const bp_table::bp_lines& lines)
   {
     int retval = 0;
 
@@ -675,21 +688,12 @@
 
             event_manager& evmgr = interp.get_event_manager ();
 
-            octave_idx_type len = line.size ();
-
-            for (int i = 0; i < len; i++)
+            for (const auto& lineno : lines)
               {
-                const_intmap_iterator p = line.find (i);
+                cmds->delete_breakpoint (lineno);
 
-                if (p != line.end ())
-                  {
-                    int lineno = p->second;
-
-                    cmds->delete_breakpoint (lineno);
-
-                    if (! file.empty ())
-                      evmgr.update_breakpoint (false, file, lineno);
-                  }
+                if (! file.empty ())
+                  evmgr.update_breakpoint (false, file, lineno);
               }
 
             results = cmds->list_breakpoints ();
@@ -705,16 +709,22 @@
     return retval;
   }
 
+  int bp_table::remove_breakpoint (const std::string& fname, int line)
+  {
+    bp_lines line_info;
+    line_info.insert (line);
+
+    return remove_breakpoint (fname, line_info);
+  }
+
   int bp_table::remove_breakpoint (const std::string& fname,
-                                   const bp_table::intmap& line)
+                                   const bp_table::bp_lines& lines)
   {
     int retval = 0;
 
-    octave_idx_type len = line.size ();
-
-    if (len == 0)
+    if (lines.empty ())
       {
-        intmap results = remove_all_breakpoints_in_file (fname);
+        bp_lines results = remove_all_breakpoints_in_file (fname);
         retval = results.size ();
       }
     else
@@ -725,7 +735,7 @@
           error ("remove_breakpoint: unable to find function %s\n",
                  fname.c_str ());
 
-        retval = remove_breakpoint_1 (dbg_fcn, fname, line);
+        retval = remove_breakpoint_1 (dbg_fcn, fname, lines);
 
         // Search subfunctions in the order they appear in the file.
 
@@ -743,7 +753,7 @@
               {
                 octave_user_code *dbg_subfcn = q->second.user_code_value ();
 
-                retval += remove_breakpoint_1 (dbg_subfcn, fname, line);
+                retval += remove_breakpoint_1 (dbg_subfcn, fname, lines);
               }
           }
       }
@@ -755,11 +765,11 @@
 
   // Remove all breakpoints from a file, including those in subfunctions.
 
-  bp_table::intmap
+  bp_table::bp_lines
   bp_table::remove_all_breakpoints_in_file (const std::string& fname,
                                             bool silent)
   {
-    intmap retval;
+    bp_lines retval;
 
     octave_user_code *dbg_fcn = m_evaluator.get_user_code (fname);
 
--- a/libinterp/parse-tree/bp-table.h	Sun Mar 21 10:53:11 2021 +0100
+++ b/libinterp/parse-tree/bp-table.h	Wed Mar 03 23:57:31 2021 -0500
@@ -61,13 +61,13 @@
 
     ~bp_table (void) = default;
 
-    // mapping from (FIXME: arbitrary index??) to line number of breakpoint
-    typedef std::map<int, int> intmap;
+    // Set of breakpoint lines.
+    typedef std::set<int> bp_lines;
 
-    typedef intmap::const_iterator const_intmap_iterator;
-    typedef intmap::iterator intmap_iterator;
+    typedef bp_lines::const_iterator const_bp_lines_iterator;
+    typedef bp_lines::iterator bp_lines_iterator;
 
-    typedef std::map <std::string, intmap> fname_line_map;
+    typedef std::map <std::string, bp_lines> fname_line_map;
 
     typedef fname_line_map::const_iterator const_fname_line_map_iterator;
     typedef fname_line_map::iterator fname_line_map_iterator;
@@ -77,17 +77,25 @@
     typedef fname_bp_map::iterator fname_bp_map_iterator;
 
     // Add a breakpoint at the nearest executable line.
-    intmap add_breakpoint (const std::string& fname = "",
+    int add_breakpoint (const std::string& fname = "",
+                        const std::string& class_name = "",
+                        int line = 1, const std::string& condition = "");
+
+    // Add a set of breakpoints at the nearest executable lines.
+    bp_lines add_breakpoint (const std::string& fname = "",
                            const std::string& class_name = "",
-                           const intmap& lines = intmap (),
+                           const bp_lines& lines = bp_lines (),
                            const std::string& condition = "");
 
-    // Remove a breakpoint from a line in file.
+    // Remove a breakpoint from the given line in file.
+    int remove_breakpoint (const std::string& fname = "", int line = 1);
+
+    // Remove a set of breakpoints from the given lines in file.
     int remove_breakpoint (const std::string& fname = "",
-                           const intmap& lines = intmap ());
+                           const bp_lines& lines = bp_lines ());
 
     // Remove all the breakpoints in a specified file.
-    intmap remove_all_breakpoints_in_file (const std::string& fname,
+    bp_lines remove_all_breakpoints_in_file (const std::string& fname,
                                            bool silent = false);
 
     // Remove all the breakpoints registered with octave.
@@ -127,7 +135,7 @@
 
     void parse_dbfunction_params (const char *who, const octave_value_list& args,
                                   std::string& func_name, std::string& class_name,
-                                  bp_table::intmap& lines, std::string& cond);
+                                  bp_table::bp_lines& lines, std::string& cond);
 
   private:
 
@@ -155,14 +163,14 @@
                           std::set<std::string>& id_list);
 
     bool add_breakpoint_1 (octave_user_code *fcn, const std::string& fname,
-                           const intmap& line, const std::string& condition,
-                           intmap& retval);
+                           const bp_lines& line, const std::string& condition,
+                           bp_lines& retval);
 
     int remove_breakpoint_1 (octave_user_code *fcn, const std::string&,
-                             const intmap& lines);
+                             const bp_lines& lines);
 
-    intmap remove_all_breakpoints_in_file_1 (octave_user_code *fcn,
-                                             const std::string& fname);
+    bp_lines remove_all_breakpoints_in_file_1 (octave_user_code *fcn,
+                                               const std::string& fname);
   };
 }
 
--- a/libinterp/parse-tree/pt-stmt.cc	Sun Mar 21 10:53:11 2021 +0100
+++ b/libinterp/parse-tree/pt-stmt.cc	Wed Mar 03 23:57:31 2021 -0500
@@ -249,49 +249,45 @@
   // Updates GUI via  event_manager::update_breakpoint.
   // FIXME: COME BACK TO ME.
 
-  bp_table::intmap
+  bp_table::bp_lines
   tree_statement_list::add_breakpoint (event_manager& evmgr,
                                        const std::string& file,
-                                       const bp_table::intmap& line,
+                                       const bp_table::bp_lines& lines,
                                        const std::string& condition)
   {
-    bp_table::intmap retval;
-
-    octave_idx_type len = line.size ();
+    bp_table::bp_lines retval;
 
-    for (int i = 0; i < len; i++)
+    for (const auto& lineno : lines)
       {
-        bp_table::const_intmap_iterator p = line.find (i);
+        int line = set_breakpoint (lineno, condition);
 
-        if (p != line.end ())
+        if (line)
           {
-            int lineno = p->second;
+            if (! file.empty ())
+              evmgr.update_breakpoint (true, file, line, condition);
 
-            retval[i] = set_breakpoint (lineno, condition);
-
-            if (retval[i] != 0 && ! file.empty ())
-              evmgr.update_breakpoint (true, file, retval[i], condition);
+            retval.insert (line);
           }
       }
 
     return retval;
   }
 
-  bp_table::intmap
+  bp_table::bp_lines
   tree_statement_list::remove_all_breakpoints (event_manager& evmgr,
                                                const std::string& file)
   {
-    bp_table::intmap retval;
+    bp_table::bp_lines retval;
 
     octave_value_list bkpts = list_breakpoints ();
 
     for (int i = 0; i < bkpts.length (); i++)
       {
-        int lineno = static_cast<int> (bkpts(i).int_value ());
+        int lineno = bkpts(i).int_value ();
 
         delete_breakpoint (lineno);
 
-        retval[i] = lineno;
+        retval.insert (lineno);
 
         if (! file.empty ())
           evmgr.update_breakpoint (false, file, lineno);
--- a/libinterp/parse-tree/pt-stmt.h	Sun Mar 21 10:53:11 2021 +0100
+++ b/libinterp/parse-tree/pt-stmt.h	Wed Mar 03 23:57:31 2021 -0500
@@ -188,13 +188,13 @@
 
     std::list<bp_type> breakpoints_and_conds (void);
 
-    bp_table::intmap add_breakpoint (event_manager& evmgr,
-                                     const std::string& file,
-                                     const bp_table::intmap& line,
-                                     const std::string& condition);
+    bp_table::bp_lines add_breakpoint (event_manager& evmgr,
+                                       const std::string& file,
+                                       const bp_table::bp_lines& lines,
+                                       const std::string& condition);
 
-    bp_table::intmap remove_all_breakpoints (event_manager& evmgr,
-                                             const std::string& file);
+    bp_table::bp_lines remove_all_breakpoints (event_manager& evmgr,
+                                               const std::string& file);
 
     void accept (tree_walker& tw)
     {