changeset 32434:9296e2b74d0b

Overhaul handling of breakpoints in classdef methods (bug #46451). * libinterp/parse-tree/bp-table.h (bp_table::add_breakpoint_in_function, bp_table::add_breakpoints_in_function, bp_table::remove_breakpoint_from_function, bp_table::remove_breakpoints_from_function): Remove separate argument for class name. Use identifiers like "@class_name/method_name" instead. * libinterp/parse-tree/bp-table.cc (user_code_provider): Extract class name from function identifier. (bp_table::add_breakpoint_in_file, bp_table::add_breakpoints_in_file): Create function identifier for method in class. (remaining functions): Adapt for those changes. Rename argument "fname" to "fcn_ident" for clarity. * libinterp/parse-tree/pt-eval.cc, pt-eval.h (tree_evaluator::get_user_code): Remove input argument "class_name". Use similar code and identifiers to get methods of classdef and legacy classes. * libinterp/corefcn/debug.cc (Fdbstop, Fdbclear): Create function identifier for method in class. * libinterp/fcn-info.cc (out_of_date_check): Remove class_name argument of "remove_all_breakpoints_from_function".
author Markus Mützel <markus.muetzel@gmx.de>
date Sun, 15 Oct 2023 17:42:49 +0200
parents c1bffa0c88db
children c082a1a49c7d
files libinterp/corefcn/debug.cc libinterp/corefcn/fcn-info.cc libinterp/parse-tree/bp-table.cc libinterp/parse-tree/bp-table.h libinterp/parse-tree/pt-eval.cc libinterp/parse-tree/pt-eval.h
diffstat 6 files changed, 129 insertions(+), 88 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/debug.cc	Thu Oct 26 10:38:28 2023 -0400
+++ b/libinterp/corefcn/debug.cc	Sun Oct 15 17:42:49 2023 +0200
@@ -189,8 +189,14 @@
 
       if (symbol_name != "")
         {
-          retmap = bptab.add_breakpoints_in_function (symbol_name, class_name,
-                   lines, condition);
+          std::string fcn_ident;
+          if (class_name.empty ())
+            fcn_ident = symbol_name;
+          else
+            fcn_ident = "@" + class_name + "/" + symbol_name;
+
+          retmap = bptab.add_breakpoints_in_function (fcn_ident, lines,
+                                                      condition);
           retval = bp_lines_to_ov (retmap);
         }
     }
@@ -242,7 +248,7 @@
               lines.clear ();
               lines.insert (line(i).int_value ());
               bptab.add_breakpoints_in_function (name(i).string_value (),
-                                                 "", lines,
+                                                 lines,
                                                  (use_cond
                                                   ? cond(i).string_value ()
                                                   : unconditional));
@@ -318,10 +324,15 @@
       bptab.remove_all_breakpoints ();
       bptab.dbclear_all_signals ();
     }
-  else
+  else if (symbol_name != "")
     {
-      if (symbol_name != "")
-        bptab.remove_breakpoints_from_function (symbol_name, class_name, lines);
+      std::string fcn_ident;
+      if (class_name.empty ())
+        fcn_ident = symbol_name;
+      else
+        fcn_ident = "@" + class_name + "/" + symbol_name;
+
+      bptab.remove_breakpoints_from_function (fcn_ident, lines);
     }
 
   // If we remove a breakpoint, we also need to reset debug_mode.
@@ -538,13 +549,27 @@
 %!   dbstop @audioplayer/set 75;
 %!   dbstop quantile>__quantile__;
 %!   dbstop ls;
-%!   s = dbstatus;
+%!   dbstop in inputParser at addOptional;
+%!   dbstop in inputParser at 285;
+%!   s = dbstatus ();
 %!   dbclear all;
+%!   ## For Matlab compatibility, the following name should be:
+%!   ## audioplayer.set>setproperty
 %!   assert (s(1).name, "@audioplayer/set>setproperty");
+%!   ## For Matlab compatibility, the following name should be:
+%!   ## ftp.dir
 %!   assert (s(2).name, "@ftp/dir");
-%!   assert (s(3).name, "ls");
-%!   assert (s(4).name, "quantile>__quantile__");
 %!   assert (s(2).file(end-10:end), [filesep "@ftp" filesep "dir.m"]);
+%!   ## For Matlab compatibility, the following two names should be:
+%!   ## inputParser.inputParser>inputParser.addOptional
+%!   assert (s(3).name, "@inputParser/addOptional");
+%!   assert (s(3).line, 278);
+%!   assert (s(4).name, "@inputParser/addOptional");
+%!   assert (s(4).line, 285);
+%!   assert (s(5).name, "ls");
+%!   assert (s(6).name, "quantile>__quantile__");
+%!   s = dbstatus ();
+%!   assert (isempty (s));
 %! unwind_protect_cleanup
 %!   if (isguirunning ())
 %!     __event_manager_gui_preference__ ("editor/show_dbg_file", orig_show_dbg);
--- a/libinterp/corefcn/fcn-info.cc	Thu Oct 26 10:38:28 2023 -0400
+++ b/libinterp/corefcn/fcn-info.cc	Sun Oct 15 17:42:49 2023 +0200
@@ -644,7 +644,7 @@
                       bp_table& bptab = __get_bp_table__ ();
 
                       bptab.remove_all_breakpoints_from_function (canonical_nm,
-                          "", true);
+                                                                  true);
                     }
                 }
             }
--- a/libinterp/parse-tree/bp-table.cc	Thu Oct 26 10:38:28 2023 -0400
+++ b/libinterp/parse-tree/bp-table.cc	Sun Oct 15 17:42:49 2023 +0200
@@ -232,7 +232,7 @@
 // Record in m_bp_set that fname contains a breakpoint.
 
 bool bp_table::add_breakpoint_1 (octave_user_code *fcn,
-                                 const std::string& fname,
+                                 const std::string& fcn_ident,
                                  const bp_table::bp_lines& line,
                                  const std::string& condition,
                                  bp_table::bp_lines& retval)
@@ -259,11 +259,12 @@
               // Otherwise, there can be an entry for both
               // file>subfunction and file, which causes a crash on
               // dbclear all
-              const char *s = strchr (fname.c_str (), '>');
+              const char *s = strchr (fcn_ident.c_str (), '>');
               if (s)
-                m_bp_set.insert (fname.substr (0, s - fname.c_str ()));
+                m_bp_set.insert (fcn_ident.substr (0, s - fcn_ident.c_str ()));
               else
-                m_bp_set.insert (fname);
+                m_bp_set.insert (fcn_ident);
+
               found = true;
               break;
             }
@@ -668,18 +669,17 @@
   return retval;
 }
 
-// Given file name fname, find the subfunction at line and create
+// Given function identifier fcn_ident, find the subfunction at line and create
 // a breakpoint there.  Put the system into debug_mode.
-int bp_table::add_breakpoint_in_function (const std::string& fname,
-    const std::string& class_name,
-    int line,
-    const std::string& condition)
+int bp_table::add_breakpoint_in_function (const std::string& fcn_ident,
+                                          int line,
+                                          const std::string& condition)
 {
   bp_lines line_info;
   line_info.insert (line);
 
   bp_lines result
-    = add_breakpoints_in_function (fname, class_name, line_info, condition);
+    = add_breakpoints_in_function (fcn_ident, line_info, condition);
 
   return result.empty () ? 0 : *(result.begin ());
 }
@@ -691,28 +691,35 @@
 class user_code_provider
 {
 public:
-  user_code_provider (const std::string& fname, octave_user_code* pfcn)
+  user_code_provider (const std::string& fcn_ident, octave_user_code* pfcn)
     : m_fcn (nullptr), m_is_valid (false)
   {
-    if (pfcn)
+    m_fcn = pfcn;
+    if (m_fcn && ! (m_fcn->is_classdef_method ()
+                    || m_fcn->is_classdef_constructor ()))
       {
         // Already have the usercode, no need to do anything.
-        m_fcn = pfcn;
         m_is_valid = true;
+        return;
       }
+
+    // If we get here, try to get a classdef to support getting method by
+    // line number.
+
+    // Extract class name from function identifier
+    std::string class_name = fcn_ident;
+    const char *s = strchr (fcn_ident.c_str (), '/');
+    if (s && fcn_ident[0] == '@')
+      class_name = fcn_ident.substr (1, s - fcn_ident.c_str () - 1);
+
+    m_cls = lookup_class (class_name, false);
+    m_is_valid = m_cls.ok () && (m_cls.get_name () == class_name);
+    if (m_is_valid)
+      populate_function_cache ();
     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 ());
-      }
+      error ("add_breakpoints_in_function: unable to find function '%s'\n",
+             fcn_ident.c_str ());
+
   }
 
   octave_user_code * operator () (int line = 1)
@@ -726,7 +733,7 @@
     return m_cls.get_method (line).user_code_value (true);
   }
 
-  bool is_function () const { return m_fcn != nullptr; }
+  bool is_function () const { return m_fcn; }
 
   bool is_valid () const { return m_is_valid; }
 
@@ -783,13 +790,12 @@
 // 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_breakpoints_in_function (const std::string& fname,
-                                       const std::string& class_name,
+bp_table::add_breakpoints_in_function (const std::string& fcn_ident,
                                        const bp_table::bp_lines& lines,
                                        const std::string& condition)
 {
   user_code_provider
-    user_code (fname, m_evaluator.get_user_code (fname, class_name));
+    user_code (fcn_ident, m_evaluator.get_user_code (fcn_ident));
 
   condition_valid (condition);  // Throw error if condition not valid.
 
@@ -804,7 +810,15 @@
       line_info.insert (lineno);
 
       bp_lines ret_one;
-      if (dbg_fcn && add_breakpoint_1 (dbg_fcn, fname, line_info,
+
+      std::string ident = fcn_ident;
+      if (! user_code.is_function () && fcn_ident[0] != '@' && dbg_fcn)
+        {
+          // identifier of the form @class_name/method_name
+          ident = "@" + fcn_ident + "/" + dbg_fcn->name ();
+        }
+
+      if (dbg_fcn && add_breakpoint_1 (dbg_fcn, ident, line_info,
                                        condition, ret_one))
         {
           if (! ret_one.empty ())
@@ -834,8 +848,13 @@
   if (! info.ok ())
     return 0;
 
-  return add_breakpoint_in_function (info.fcn (), info.class_name (),
-                                     line, condition);
+  std::string fcn_ident;
+  if (info.class_name ().empty ())
+    fcn_ident = info.fcn ();
+  else
+    fcn_ident = "@" + info.class_name () + "/" + info.fcn ();
+
+  return add_breakpoint_in_function (fcn_ident, line, condition);
 }
 
 bp_table::bp_lines
@@ -851,12 +870,17 @@
   if (! info.ok ())
     return bp_lines ();
 
-  return add_breakpoints_in_function (info.fcn (), info.class_name (),
-                                      lines, condition);
+  std::string fcn_ident;
+  if (info.class_name ().empty ())
+    fcn_ident = info.fcn ();
+  else
+    fcn_ident = "@" + info.class_name () + "/" + info.fcn ();
+
+  return add_breakpoints_in_function (fcn_ident, lines, condition);
 }
 
 int bp_table::remove_breakpoint_1 (octave_user_code *fcn,
-                                   const std::string& fname,
+                                   const std::string& fcn_ident,
                                    const bp_table::bp_lines& lines)
 {
   int retval = 0;
@@ -887,7 +911,7 @@
 
           results = cmds->list_breakpoints ();
 
-          auto it = m_bp_set.find (fname);
+          auto it = m_bp_set.find (fcn_ident);
           if (results.empty () && it != m_bp_set.end ())
             m_bp_set.erase (it);
         }
@@ -899,31 +923,30 @@
 }
 
 int
-bp_table::remove_breakpoint_from_function (const std::string& fname,
-                                           const std::string& cname, int line)
+bp_table::remove_breakpoint_from_function (const std::string& fcn_ident,
+                                           int line)
 {
   bp_lines line_info;
   line_info.insert (line);
 
-  return remove_breakpoints_from_function (fname, cname, line_info);
+  return remove_breakpoints_from_function (fcn_ident, line_info);
 }
 
 int
-bp_table::remove_breakpoints_from_function (const std::string& fname,
-                                            const std::string& cname,
+bp_table::remove_breakpoints_from_function (const std::string& fcn_ident,
                                             const bp_table::bp_lines& lines)
 {
   int retval = 0;
 
   if (lines.empty ())
     {
-      bp_lines results = remove_all_breakpoints_from_function (fname, cname);
+      bp_lines results = remove_all_breakpoints_from_function (fcn_ident);
       retval = results.size ();
     }
   else
     {
-      octave_user_code *dbg_fcn = m_evaluator.get_user_code (fname, cname);
-      user_code_provider user_code (fname, dbg_fcn);
+      octave_user_code *dbg_fcn = m_evaluator.get_user_code (fcn_ident);
+      user_code_provider user_code (fcn_ident, dbg_fcn);
 
       if (user_code.is_valid ())
         {
@@ -970,21 +993,21 @@
                       octave_user_code *dbg_subfcn
                         = q->second.user_code_value ();
 
-                      retval += remove_breakpoint_1 (dbg_subfcn, fname, lines);
+                      retval += remove_breakpoint_1 (dbg_subfcn, fcn_ident, lines);
                     }
                 }
             }
           // Remove file from breakpoint set if no breakpoints remain
-          octave_value_list fname_list = {fname};
+          octave_value_list fname_list = {fcn_ident};
           const bool no_breakpoints
             = get_breakpoint_list (fname_list).empty ();
-          auto iter = m_bp_set.find (fname);
+          auto iter = m_bp_set.find (fcn_ident);
           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 ());
+               fcn_ident.c_str ());
     }
 
   m_evaluator.reset_debug_state ();
@@ -995,14 +1018,13 @@
 // Remove all breakpoints from a file, including those in subfunctions.
 
 bp_table::bp_lines
-bp_table::remove_all_breakpoints_from_function (const std::string& fname,
-                                                const std::string& cname,
+bp_table::remove_all_breakpoints_from_function (const std::string& fcn_ident,
                                                 bool silent)
 {
   bp_lines retval;
 
-  octave_user_code *fcn = m_evaluator.get_user_code (fname, cname);
-  user_code_provider user_code (fname, fcn);
+  octave_user_code *fcn = m_evaluator.get_user_code (fcn_ident);
+  user_code_provider user_code (fcn_ident, fcn);
 
   if (user_code.is_valid ())
     {
@@ -1022,13 +1044,13 @@
               retval = cmds->remove_all_breakpoints (evmgr, file);
             }
         }
-      auto it = m_bp_set.find (fname);
+      auto it = m_bp_set.find (fcn_ident);
       if (it != m_bp_set.end ())
         m_bp_set.erase (it);
     }
   else if (! silent)
     error ("remove_all_breakpoints_from_function: "
-           "unable to find function %s\n", fname.c_str ());
+           "unable to find function %s\n", fcn_ident.c_str ());
 
   m_evaluator.reset_debug_state ();
 
@@ -1046,7 +1068,7 @@
   if (! info.ok ())
     return 0;
 
-  return remove_breakpoint_from_function (info.fcn (), "", line);
+  return remove_breakpoint_from_function (info.fcn (), line);
 }
 
 int
@@ -1061,7 +1083,7 @@
   if (! info.ok ())
     return 0;
 
-  return remove_breakpoints_from_function (info.fcn (), "", lines);
+  return remove_breakpoints_from_function (info.fcn (), lines);
 }
 
 bp_table::bp_lines
@@ -1076,7 +1098,7 @@
   if (! info.ok ())
     return bp_lines ();
 
-  return remove_all_breakpoints_from_function (info.fcn (), "", silent);
+  return remove_all_breakpoints_from_function (info.fcn (), silent);
 }
 
 void bp_table::remove_all_breakpoints ()
@@ -1147,8 +1169,8 @@
                                                    bp.cend ());
                         }
                     }
+                }
 
-                }
               if (! all_bkpts.empty ())
                 retval[bp_fname] = all_bkpts;
             }
--- a/libinterp/parse-tree/bp-table.h	Thu Oct 26 10:38:28 2023 -0400
+++ b/libinterp/parse-tree/bp-table.h	Sun Oct 15 17:42:49 2023 +0200
@@ -82,15 +82,13 @@
   typedef fname_bp_map::iterator fname_bp_map_iterator;
 
   // Add a breakpoint at the nearest executable line in a function.
-  int add_breakpoint_in_function (const std::string& fname = "",
-                                  const std::string& class_name = "",
+  int add_breakpoint_in_function (const std::string& fcn_ident = "",
                                   int line = 1,
                                   const std::string& condition = "");
 
   // Add a set of breakpoints at the nearest executable lines in a
   // function.
-  bp_lines add_breakpoints_in_function (const std::string& fname = "",
-                                        const std::string& class_name = "",
+  bp_lines add_breakpoints_in_function (const std::string& fcn_ident = "",
                                         const bp_lines& lines = bp_lines (),
                                         const std::string& condition = "");
 
@@ -106,18 +104,15 @@
                                     const std::string& condition = "");
 
   // Remove a breakpoint from the given line in file.
-  int remove_breakpoint_from_function (const std::string& fname = "",
-                                       const std::string& cname = "",
+  int remove_breakpoint_from_function (const std::string& fcn_ident = "",
                                        int line = 1);
 
   // Remove a set of breakpoints from the given lines in file.
-  int remove_breakpoints_from_function (const std::string& fname = "",
-                                        const std::string& cname = "",
+  int remove_breakpoints_from_function (const std::string& fcn_ident = "",
                                         const bp_lines& lines = bp_lines ());
 
   // Remove all the breakpoints in a specified function.
-  bp_lines remove_all_breakpoints_from_function (const std::string& fname,
-                                                 const std::string& cname = "",
+  bp_lines remove_all_breakpoints_from_function (const std::string& fcn_ident,
                                                  bool silent = false);
 
   // Remove a breakpoint from the given line in file.
--- a/libinterp/parse-tree/pt-eval.cc	Thu Oct 26 10:38:28 2023 -0400
+++ b/libinterp/parse-tree/pt-eval.cc	Sun Oct 15 17:42:49 2023 +0200
@@ -2896,8 +2896,7 @@
 // current call stack.
 
 octave_user_code *
-tree_evaluator::get_user_code (const std::string& fname,
-                               const std::string& class_name)
+tree_evaluator::get_user_code (const std::string& fname)
 {
   octave_user_code *user_code = nullptr;
 
@@ -2940,17 +2939,18 @@
 
           std::string method = name.substr (p1+1, p2-1);
 
-          fcn = symtab.find_method (method, dispatch_type);
-        }
-      else if (! class_name.empty ())
-        {
+          // first check for classdef method
           cdef_manager& cdm = m_interpreter.get_cdef_manager ();
 
-          fcn = cdm.find_method (class_name, name);
+//          fcn = cdm.find_method_symbol (method, dispatch_type);
+
+          cdef_class cls = cdm.find_class (dispatch_type, false);
+          if (cls.ok () && cls.get_name () == dispatch_type)
+            fcn = cls.find_method (method).get_function ();
 
           // If there is no classdef method, then try legacy classes.
           if (fcn.is_undefined ())
-            fcn = symtab.find_method (name, class_name);
+            fcn = symtab.find_method (method, dispatch_type);
         }
       else
         {
--- a/libinterp/parse-tree/pt-eval.h	Thu Oct 26 10:38:28 2023 -0400
+++ b/libinterp/parse-tree/pt-eval.h	Sun Oct 15 17:42:49 2023 +0200
@@ -575,8 +575,7 @@
 
   std::list<std::string> variable_names () const;
 
-  octave_user_code * get_user_code (const std::string& fname = "",
-                                   const std::string& class_name = "");
+  octave_user_code * get_user_code (const std::string& fname = "");
 
   std::string current_function_name (bool skip_first = false) const;