Re: [RFC] Debug Methods in GDB Python

[Tom Tromey <tromey@redhat.com>]

>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:

>> It's fine for the functions themselves, since we'd like to keep the API
>> to the Python layer reasonably thin.  But for get_matching_ext_methods,
>> e.g., it is simple to follow the existing pattern: declare a function in
>> python.h and provide both with- and without-Python implementations.

Siva> I am not sure I understand what is being said here. Could you kindly
Siva> elaborate (pointing to an existing example)?

I think it's irrelevant now.

>> Silently ignoring errors doesn't seem right.
>> There are a few instances of this.
>> 
>> I'm not really sure about the error-handling strategy in this function.

Siva> They way I have looked at debug methods hook is as follows:
Siva> "If there exists a Python implementation or replacement which matches
Siva> better or is as good a match as the source implementation, then use
Siva> the Python implementation. If there is any error looking up Python
Siva> implementations, report the error but do not stop from executing an
Siva> operation; proceed to use the source implementation."

Siva> With that view, in the latest patch, I have modified to print the
Siva> errors instead of silently ignoring them. Let me know if this does not
Siva> seem to be a good strategy.

I think that's fine.

Swallowing exceptions is unfriendly for folks writing the Python code.
Ignoring them can be ok as long as they are printed at the point at
which they are dropped.  Then one can at least see the stack trace and
fix the bug.

Siva> +optimized out.  Lastly, one could define a debug method in @value{GDBN}
Siva> +Python which does not have an equivalent in the source language!

Very interesting.  I was going to ask about that.  I'd like to see it
done in the tests...


I think I'm ok with the overall approach.

I didn't read every single bit in detail.  So I didn't go over the
error-checking and reference counting bits the way I normally would.

I can do that if you want; but meanwhile I think if you wrote the
tests...


Also I was curious if this supports operator overloading.  Like can I
define an "operator+"?

Siva> +              if (ext_fnp)
Siva> +                {
Siva> +                  if (ext_fn_is_method (ext_fnp))
Siva> +                    {
Siva> +                      struct value *ret_val;
Siva> +
Siva> +                      ret_val = ext_fn_invoke_method (ext_fnp, arg2, argvec + 2,
Siva> +                                                      nargs - 1);
Siva> +                      if (ret_val == NULL)
Siva> +                        error (_("Error invoking debug method for method %s."),
Siva> +                               tstr);
Siva> +
Siva> +                      return ret_val;
Siva> +                    }
Siva> +                }

What happens here if "ext_fnp && !ext_fn_is_method"?

It seems like a check is needed.

Siva> +struct py_ext_object
Siva> +{
Siva> +  /* Holds an instance of the DebugMethod class.  */
Siva> +  PyObject *object;
Siva> +
Siva> +  /* Holds the type of the 'this' object.  */
Siva> +  PyObject *match_py_obj_type;
Siva> +
Siva> +  /* Holds the matching method name.  */
Siva> +  const char *match_method;

How "match_method" is allocated and how its lifetime is managed is
really unclear to me.

Tom