* [PATCH 1/4 v18] Add xmethod documentation and NEWS entry @ 2014-05-23 21:55 Siva Chandra 2014-05-24 7:13 ` Eli Zaretskii 2014-05-25 23:00 ` Doug Evans 0 siblings, 2 replies; 4+ messages in thread From: Siva Chandra @ 2014-05-23 21:55 UTC (permalink / raw) To: gdb-patches [-- Attachment #1: Type: text/plain, Size: 563 bytes --] This part was approved (I think) previously but sending it out with version synced to v18. Also, the name changed from "debug method" to "xmethod". Hence, I think another review is required. ChangeLog 2014-05-23 Siva Chandra Reddy <sivachandra@google.com> * NEWS (Python Scripting): Add entry about the new xmethods feature. doc/ * python.texi (Xmethods In Python, XMethod API) (Writing an Xmethod): New nodes. (Python API): New menu entries "Xmethods In Python", "Xmethod API", "Writing an Xmethod". [-- Attachment #2: xmethod_docs_v18.txt --] [-- Type: text/plain, Size: 11392 bytes --] diff --git a/gdb/NEWS b/gdb/NEWS index 5a0a76e..6d4ea20 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -133,6 +133,11 @@ qXfer:btrace:read's annex ** Valid Python operations on gdb.Value objects representing structs/classes invoke the corresponding overloaded operators if available. + ** New `Xmethods' feature in the Python API. Xmethods are + additional methods or replacements for existing methods of a C++ + class. This feature is useful for those cases where a method + defined in C++ source code could be inlined or optimized out by + the compiler, making it unavailable to GDB. * New targets PowerPC64 GNU/Linux little-endian powerpc64le-*-linux* diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index ba0a7fd..33680fb 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -144,6 +144,9 @@ optional arguments while skipping others. Example: * Frame Filter API:: Filtering Frames. * Frame Decorator API:: Decorating Frames. * Writing a Frame Filter:: Writing a Frame Filter. +* Xmethods In Python:: Adding and replacing methods of C++ classes. +* Xmethod API:: Xmethod types. +* Writing an Xmethod:: Writing an xmethod. * Inferiors In Python:: Python representation of inferiors (processes) * Events In Python:: Listening for events from @value{GDBN}. * Threads In Python:: Accessing inferior threads from Python. @@ -2175,6 +2178,265 @@ printed hierarchically. Another approach would be to combine the marker in the inlined frame, and also show the hierarchical relationship. +@node Xmethods In Python +@subsubsection Xmethods In Python +@cindex xmethods in Python + +@dfn{Xmethods} are additional methods or replacements for existing +methods of a C@t{++} class. This feature is useful for those cases +where a method defined in C@t{++} source code could be inlined or +optimized out by the compiler, making it unavailable to @value{GDBN}. +For such cases, one can define an xmethod to serve as a replacement +for the method defined in the C@t{++} source code. @value{GDBN} will +then invoke the xmethod, instead of the C@t{++} method, to +evaluate expressions. One can also use xmethods when debugging +with core files. Moreover, when debugging live programs, invoking an +xmethod need not involve running the inferior (which can potentially +perturb its state). Hence, even if the C@t{++} method is available, it +is better to use its replacement xmethod if one is defined. + +The xmethods feature in Python is available via the concepts of an +@dfn{xmethod matcher} and an @dfn{xmethod worker}. To +implement an xmethod, one has to implement a matcher and a +corresponding worker for it (more than one worker can be +implemented, each catering to a different overloaded instance of the +method). Internally, @value{GDBN} invokes the @code{match} method of a +matcher to match the class type and method name. On a match, the +@code{match} method returns a list of matching @emph{worker} objects. +Each worker object typically corresponds to an overloaded instance of +the xmethod. They implement a @code{get_arg_types} method which +returns a sequence of types corresponding to the arguments the xmethod +requires. @value{GDBN} uses this sequence of types to perform +overload resolution and picks a winning xmethod worker. A winner +is also selected from among the methods @value{GDBN} finds in the +C@t{++} source code. Next, the winning xmethod worker and the +winning C@t{++} method are compared to select an overall winner. In +case of a tie between a xmethod worker and a C@t{++} method, the +xmethod worker is selected as the winner. That is, if a winning +xmethod worker is found to be equivalent to the winning C@t{++} +method, then the xmethod worker is treated as a replacement for +the C@t{++} method. @value{GDBN} uses the overall winner to invoke the +method. If the winning xmethod worker is the overall winner, then +the corresponding xmethod is invoked via the @code{invoke} method +of the worker object. + +If one wants to implement an xmethod as a replacement for an +existing C@t{++} method, then they have to implement an equivalent +xmethod which has exactly the same name and takes arguments of +exactly the same type as the C@t{++} method. If the user wants to +invoke the C@t{++} method even though a replacement xmethod is +available for that method, then they can disable the xmethod. + +@xref{Xmethod API}, for API to implement xmethods in Python. +@xref{Writing an Xmethod}, for implementing xmethods in Python. + +@node Xmethod API +@subsubsection Xmethod API +@cindex xmethod API + +The @value{GDBN} Python API provides classes, interfaces and functions +to implement, register and manipulate xmethods. +@xref{Xmethods In Python}. + +A xmethod matcher should be an instance of a class derived from +@code{XMethodMatcher} defined in the module @code{gdb.xmethod}, or an +object with similar interface and attributes. An instance of +@code{XMethodMatcher} has the following attributes: + +@defvar name +The name of the matcher. +@end defvar + +@defvar enabled +A boolean value indicating whether the matcher is enabled or disabled. +@end defvar + +@defvar methods +A list of named methods managed by the matcher. Each object in the list +is an instance of the class @code{XMethod} defined in the module +@code{gdb.xmethod}, or any object with the following attributes: + +@table @code + +@item name +Name of the xmethod which should be unique for each xmethod +managed by the matcher. + +@item enabled +A boolean value indicating whether the xmethod is enabled or +disabled. + +@end table + +The class @code{XMethod} is a convenience class with same +attributes as above along with the following constructor: + +@defun XMethod.__init__(self, name) +Constructs an enabled xmethod with name @var{name}. +@end defun +@end defvar + +@noindent +The @code{XMethodMatcher} class has the following methods: + +@defun XMethodMatcher.__init__(self, name) +Constructs an enabled xmethod matcher with name @var{name}. The +@code{methods} attribute is initialized to @code{None}. +@end defun + +@defun XMethodMatcher.match(self, class_type, method_name) +Derived classes should override this method. It should return a +xmethod worker object (or a sequence of xmethod worker +objects) matching the @var{class_type} and @var{method_name}. +@var{class_type} is a @code{gdb.Type} object, and @var{method_name} +is a string value. If the matcher manages named methods as listed in +its @code{methods} attribute, then only those worker objects whose +corresponding entries in the @code{methods} list are enabled should be +returned. +@end defun + +A xmethod worker should be an instance of a class derived from +@code{XMethodWorker} defined in the module @code{gdb.xmethod}, +or support the following interface: + +@defun XMethodWorker.get_arg_types(self) +This method returns a sequence of @code{gdb.Type} objects corresponding +to the arguments that the xmethod takes. It can return an empty +sequence or @code{None} if the xmethod does not take any arguments. +If the xmethod takes a single argument, then a single +@code{gdb.Type} object corresponding to it can be returned. +@end defun + +@defun XMethodWorker.invoke(self, obj, args) +This is the method which does the @emph{work} of the xmethod. +@var{obj} is the object on which the method is being invoked, and +@var{args} is the tuple of arguments to the method. @var{obj} and the +elements of @var{args} are @code{gdb.Value} objects. +@end defun + +For @value{GDBN} to lookup xmethods, the xmethod matchers +should be registered using the following function defined in the module +@code{gdb.xmethod}: + +@defun register_xmethod_matcher(locus, matcher, replace=False) +The @code{matcher} is registered with @code{locus}, replacing an +existing matcher with the same name as @code{matcher} if +@code{replace} is @code{True}. @code{locus} can be a +@code{gdb.Objfile} object (@pxref{Objfiles In Python}), or a +@code{gdb.Progspace} object (@pxref{Program Spaces In Python}), or +@code{None}. If it is @code{None}, then @code{matcher} is registered +globally. +@end defun + +@node Writing a Xmethod +@subsubsection Writing a Xmethod +@cindex writing xmethods in Python + +Implementing xmethods in Python will require implementing xmethod +matchers and xmethod workers (@pxref{Xmethods In Python}). Consider +the following C@t{++} class: + +@smallexample +class MyClass +@{ + public: + MyClass (int a) : a_(a) {} + + int geta (void) { return a_; } + int operator+ (int b); + + private: + int a_; +@}; + +int +MyClass::operator+ (int b) +@{ + return a_ + b; +@} +@end smallexample + +@noindent +Let us define two xmethods for the class @code{MyClass}, one +replacing the method @code{geta}, and another adding an overloaded +flavor of @code{operator+} which takes a @code{MyClass} argument. The +xmethod matcher can be defined as follows: + +@smallexample +class MyClassMatcher(gdb.xmethod.XMethodMatcher): + def __init__(self): + gdb.xmethod.XMethodMatcher.__init__(self, 'MyMatcher') + # List of methods 'managed' by this matcher + self.methods = [gdb.xmethod.XMethod('geta'), + gdb.xmethod.XMethod('sum')] + + def match(self, class_type, method_name): + if class_type.tag != 'MyClass': + return None + if method_name == 'geta' and self.methods[0].enabled: + return MyClassWorker_geta() + elif method_name == 'operator+' and self.methods[1].enabled: + return MyClassWorker_plus() + else: + return None +@end smallexample + +@noindent +Notice that the @code{match} method of @code{MyClassMatcher} returns +a worker object of type @code{MyClassWorker_geta} for the @code{geta} +method, and a worker object of type @code{MyClassWorker_plus} for the +@code{operator+} method. Also, a worker object is returned only if the +corresponding entry in the @code{methods} attribute is enabled. + +The implementation of the worker classes returned by the matcher above +is as follows: + +@smallexample +class MyClassWorker_geta(gdb.xmethod.XMethodWorker): + def get_arg_types(self): + return None + + def invoke(self, obj, args): + return obj['a_'] + + +class MyClassWorker_plus(gdb.xmethod.XMethodWorker): + def get_arg_types(self): + return gdb.lookup_type('MyClass') + + def invoke(self, obj, args): + return obj['a_'] + args[0]['a_'] +@end smallexample + +For @value{GDBN} to actually lookup a xmethod, it has to be +registered with it. The matcher defined above is registered with +@value{GDBN} globally as follows: + +@smallexample +gdb.xmethod.register_xmethod_matcher(None, MyClassMatcher()) +@end smallexample + +If an object @code{obj} of type @code{MyClass} is initialized in C@t{++} +code as follows: + +@smallexample +MyClass obj(5); +@end smallexample + +@noindent +then, after loading the Python script defining the xmethod matchers +and workers into @code{GDBN}, invoking the method @code{geta} or using +the operator @code{+} on @code{obj} will invoke the xmethods +defined above: + +@smallexample +(gdb) p obj.geta() +$1 = 5 + +(gdb) p obj + obj +$2 = 10 +@end smallexample + @node Inferiors In Python @subsubsection Inferiors In Python @cindex inferiors in Python ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4 v18] Add xmethod documentation and NEWS entry 2014-05-23 21:55 [PATCH 1/4 v18] Add xmethod documentation and NEWS entry Siva Chandra @ 2014-05-24 7:13 ` Eli Zaretskii 2014-05-25 23:00 ` Doug Evans 1 sibling, 0 replies; 4+ messages in thread From: Eli Zaretskii @ 2014-05-24 7:13 UTC (permalink / raw) To: Siva Chandra; +Cc: gdb-patches > Date: Fri, 23 May 2014 14:55:01 -0700 > From: Siva Chandra <sivachandra@google.com> > > This part was approved (I think) previously but sending it out with > version synced to v18. Also, the name changed from "debug method" to > "xmethod". Hence, I think another review is required. You've got it. > +@defun XMethodWorker.invoke(self, obj, args) > +This is the method which does the @emph{work} of the xmethod. > +@var{obj} is the object on which the method is being invoked, and > +@var{args} is the tuple of arguments to the method. @var{obj} and the > +elements of @var{args} are @code{gdb.Value} objects. It is best not to begin a sentence with "@var{something}", since in the printed manual that will produce a sentence starting from a lower-case letter. Otherwise, OK. Thanks. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4 v18] Add xmethod documentation and NEWS entry 2014-05-23 21:55 [PATCH 1/4 v18] Add xmethod documentation and NEWS entry Siva Chandra 2014-05-24 7:13 ` Eli Zaretskii @ 2014-05-25 23:00 ` Doug Evans 2014-05-28 6:10 ` Doug Evans 1 sibling, 1 reply; 4+ messages in thread From: Doug Evans @ 2014-05-25 23:00 UTC (permalink / raw) To: Siva Chandra; +Cc: gdb-patches Siva Chandra <sivachandra@google.com> writes: > This part was approved (I think) previously but sending it out with > version synced to v18. Also, the name changed from "debug method" to > "xmethod". Hence, I think another review is required. > > ChangeLog > 2014-05-23 Siva Chandra Reddy <sivachandra@google.com> > > * NEWS (Python Scripting): Add entry about the new xmethods > feature. > > doc/ > * python.texi (Xmethods In Python, XMethod API) > (Writing an Xmethod): New nodes. > (Python API): New menu entries "Xmethods In Python", > "Xmethod API", "Writing an Xmethod". Hi. It was easiest to make comments by editing the code, adding FIXMEs and whatnot. // comments are comments for discussion, not intended to be checked in. I'll apply the relevant comments to pieces 2,3,4 too. commit 20b935157b64ae20d251bd4e4d5f1c111ffdb868 Author: Doug Evans <xdje42@gmail.com> Date: Sun May 25 15:54:45 2014 -0700 review comments on xmethods v18 patch diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index 72d58ad..026790f 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -2237,7 +2237,7 @@ The @value{GDBN} Python API provides classes, interfaces and functions to implement, register and manipulate xmethods. @xref{Xmethods In Python}. -A xmethod matcher should be an instance of a class derived from +An xmethod matcher should be an instance of a class derived from @code{XMethodMatcher} defined in the module @code{gdb.xmethod}, or an object with similar interface and attributes. An instance of @code{XMethodMatcher} has the following attributes: @@ -2294,7 +2294,7 @@ corresponding entries in the @code{methods} list are enabled should be returned. @end defun -A xmethod worker should be an instance of a class derived from +An xmethod worker should be an instance of a class derived from @code{XMethodWorker} defined in the module @code{gdb.xmethod}, or support the following interface: @@ -2327,8 +2327,8 @@ existing matcher with the same name as @code{matcher} if globally. @end defun -@node Writing a Xmethod -@subsubsection Writing a Xmethod +@node Writing an Xmethod +@subsubsection Writing an Xmethod @cindex writing xmethods in Python Implementing xmethods in Python will require implementing xmethod @@ -2372,6 +2372,11 @@ class MyClassMatcher(gdb.xmethod.XMethodMatcher): def match(self, class_type, method_name): if class_type.tag != 'MyClass': return None + # FIXME: User code shouldn't have to check the enabled flag. + # py-xmethods.c checks it, so can this check just be removed? + # If user code wants to add this enabled check, e.g., should a perf + # concern arise, then ok; but we shouldn't encourage it as the default + # mode of writing xmethods. if method_name == 'geta' and self.methods[0].enabled: return MyClassWorker_geta() elif method_name == 'operator+' and self.methods[1].enabled: @@ -2386,6 +2391,7 @@ a worker object of type @code{MyClassWorker_geta} for the @code{geta} method, and a worker object of type @code{MyClassWorker_plus} for the @code{operator+} method. Also, a worker object is returned only if the corresponding entry in the @code{methods} attribute is enabled. +@c FIXME: See above: User code shouldn't check enabled flag, gdb should. The implementation of the worker classes returned by the matcher above is as follows: @@ -2403,6 +2409,9 @@ class MyClassWorker_plus(gdb.xmethod.XMethodWorker): def get_arg_types(self): return gdb.lookup_type('MyClass') + # FIXME: IWBN if the invoke function could spell out the args instead + # of just having "args" as a list of all of them. + # If the user wants the args as a list then can write *args here, right? def invoke(self, obj, args): return obj['a_'] + args[0]['a_'] @end smallexample diff --git a/gdb/eval.c b/gdb/eval.c index d374b6a..2909420 100644 --- a/gdb/eval.c +++ b/gdb/eval.c @@ -1771,6 +1771,15 @@ evaluate_subexp_standard (struct type *expect_type, return call_internal_function (exp->gdbarch, exp->language_defn, argvec[0], nargs, argvec + 1); case TYPE_CODE_XMETHOD: + // If call_internal_function gets gdbarch,language here a reasonable + // case can be made that call_xmethod should too. gdb has too much + // global state, we should make a point of passing things explicitly + // when we can, especially if similar code already is. + // I suspect more state than gdbarch,language could be required, + // and it would be better to create an object (or preferably use an + // existing one) that contains all the info. + // Whether it is important enough to make this change now ... dunno. + // I'd say leave this as is. Just wanted to write this down. return call_xmethod (argvec[0], nargs, argvec + 1); default: return call_function_by_hand (argvec[0], nargs, argvec + 1); diff --git a/gdb/python/lib/gdb/command/xmethods.py b/gdb/python/lib/gdb/command/xmethods.py index f61e7fb..31f9cdd 100644 --- a/gdb/python/lib/gdb/command/xmethods.py +++ b/gdb/python/lib/gdb/command/xmethods.py @@ -241,7 +241,7 @@ class DisableXMethod(gdb.Command): Usage: disable xmethod [locus-regexp [name-regexp]] LOCUS-REGEXP is a regular expression matching the location of the - xmethod matcherss. If it is omitted, all registered xmethod matchers + xmethod matchers. If it is omitted, all registered xmethod matchers from all loci are disabled. A locus could be 'global', a regular expression matching the current program space's filename, or a regular expression filenames of objfiles. Locus could be 'progspace' to specify diff --git a/gdb/python/lib/gdb/xmethod.py b/gdb/python/lib/gdb/xmethod.py index 9d0deff..124625e 100644 --- a/gdb/python/lib/gdb/xmethod.py +++ b/gdb/python/lib/gdb/xmethod.py @@ -109,7 +109,7 @@ class XMethodWorker(object): argument, then a gdb.Type object or a sequence with a single gdb.Type element is returned. """ - raise NotImplementedError("XMethod get_arg_types") + raise NotImplementedError("XMethodWorker get_arg_types") def invoke(self, obj, args): """Invoke the xmethod. @@ -124,7 +124,7 @@ class XMethodWorker(object): A gdb.Value corresponding to the value returned by the xmethod. Returns 'None' if the method does not return anything. """ - raise NotImplementedError("XMethod invoke") + raise NotImplementedError("XMethodWorker invoke") class SimpleXMethodMatcher(XMethodMatcher): @@ -192,7 +192,7 @@ class SimpleXMethodMatcher(XMethodMatcher): # object if MATCHER is not having the requisite attributes in the proper # format. -def validate_xmethod_matcher(matcher): +def _validate_xmethod_matcher(matcher): if not hasattr(matcher, "match"): return TypeError("Xmethod matcher is missing method: match") if not hasattr(matcher, "name"): @@ -210,7 +210,7 @@ def validate_xmethod_matcher(matcher): # xmethod matcher with NAME in LOCUS. Returns the index of the xmethod # matcher in 'xmethods' sequence attribute of the LOCUS. -def lookup_xmethod_matcher(locus, name): +def _lookup_xmethod_matcher(locus, name): i = 0 for m in locus.xmethods: if m.name == name: @@ -233,7 +233,7 @@ def register_xmethod_matcher(locus, matcher, replace=False): same name in the locus. Otherwise, if a matcher with the same name exists in the locus, raise an exception. """ - err = validate_xmethod_matcher(matcher) + err = _validate_xmethod_matcher(matcher) if err: raise err if not locus: @@ -242,7 +242,7 @@ def register_xmethod_matcher(locus, matcher, replace=False): locus_name = "global" else: locus_name = locus.filename - index = lookup_xmethod_matcher(locus, matcher.name) + index = _lookup_xmethod_matcher(locus, matcher.name) if index: if replace: del locus.xmethods[index] diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c index 0868c9f..7e725b5 100644 --- a/gdb/python/py-xmethods.c +++ b/gdb/python/py-xmethods.c @@ -49,34 +49,46 @@ static struct xmethod_worker *new_python_xmethod_worker /* Implementation of free_xmethod_worker_data for Python. */ void -gdbpy_free_xmethod_worker_data - (const struct extension_language_defn *extlang, void *data) +gdbpy_free_xmethod_worker_data (const struct extension_language_defn *extlang, + void *data) { struct gdbpy_worker_data *worker_data = data; + struct cleanup *cleanups; gdb_assert (worker_data->worker != NULL && worker_data->this_type != NULL); + /* We don't do much here, but we still need the GIL. */ + cleanups = ensure_python_env (get_current_arch (), current_language); + Py_DECREF (worker_data->worker); Py_DECREF (worker_data->this_type); xfree (worker_data); + + do_cleanups (cleanups); } /* Implementation of clone_xmethod_worker_data for Python. */ void * -gdbpy_clone_xmethod_worker_data - (const struct extension_language_defn *extlang, void *data) +gdbpy_clone_xmethod_worker_data (const struct extension_language_defn *extlang, + void *data) { struct gdbpy_worker_data *worker_data = data, *new_data; + struct cleanup *cleanups; gdb_assert (worker_data->worker != NULL && worker_data->this_type != NULL); + /* We don't do much here, but we still need the GIL. */ + cleanups = ensure_python_env (get_current_arch (), current_language); + new_data = XCNEW (struct gdbpy_worker_data); new_data->worker = worker_data->worker; new_data->this_type = worker_data->this_type; Py_INCREF (new_data->worker); Py_INCREF (new_data->this_type); + do_cleanups (cleanups); + return new_data; } @@ -180,7 +192,9 @@ gdbpy_get_matching_xmethod_workers return EXT_LANG_RC_ERROR; } - /* Gather debug method matchers registered with the object files. */ + /* Gather debug method matchers registered with the object files. + This could be done differently by iterating over each objfile's matcher + list individually, but there's no data yet to show it's needed. */ ALL_OBJFILES (objfile) { PyObject *py_objfile = objfile_to_objfile_object (objfile); @@ -196,8 +210,7 @@ gdbpy_get_matching_xmethod_workers } objfile_matchers = objfpy_get_xmethods (py_objfile, NULL); - py_xmethod_matcher_list = PySequence_Concat (temp, - objfile_matchers); + py_xmethod_matcher_list = PySequence_Concat (temp, objfile_matchers); Py_DECREF (temp); Py_DECREF (objfile_matchers); if (py_xmethod_matcher_list == NULL) @@ -248,8 +261,7 @@ gdbpy_get_matching_xmethod_workers matchers_attr_str); if (gdb_matchers != NULL) { - py_xmethod_matcher_list = PySequence_Concat (temp, - gdb_matchers); + py_xmethod_matcher_list = PySequence_Concat (temp, gdb_matchers); Py_DECREF (temp); Py_DECREF (gdb_matchers); if (py_xmethod_matcher_list == NULL) @@ -362,8 +374,8 @@ gdbpy_get_matching_xmethod_workers enum ext_lang_rc gdbpy_get_xmethod_arg_types (const struct extension_language_defn *extlang, - struct xmethod_worker *worker, - int *nargs, struct type ***arg_types) + struct xmethod_worker *worker, + int *nargs, struct type ***arg_types) { struct gdbpy_worker_data *worker_data = worker->data; PyObject *py_worker = worker_data->worker; @@ -457,13 +469,15 @@ gdbpy_get_xmethod_arg_types (const struct extension_language_defn *extlang, struct type *arg_type = type_object_to_type (py_argtype_list); if (arg_type == NULL) - PyErr_SetString (PyExc_TypeError, - _("Arg type returned by the get_arg_types method " - "of a debug method worker object is not a gdb.Type " - "object.")); + { + PyErr_SetString (PyExc_TypeError, + _("Arg type returned by the get_arg_types method " + "of a debug method worker object is not a " + "gdb.Type object.")); + } else { - type_array[1] = arg_type; + type_array[i] = arg_type; i++; } } @@ -478,6 +492,7 @@ gdbpy_get_xmethod_arg_types (const struct extension_language_defn *extlang, /* Add the type of 'this' as the first argument. */ obj_type = type_object_to_type (worker_data->this_type); + /* FIXME: Add comment explaining why passing 1 for "is const" is ok here. */ type_array[0] = make_cv_type (1, 0, lookup_pointer_type (obj_type), NULL); *nargs = i; *arg_types = type_array; @@ -490,9 +505,9 @@ gdbpy_get_xmethod_arg_types (const struct extension_language_defn *extlang, enum ext_lang_rc gdbpy_invoke_xmethod (const struct extension_language_defn *extlang, - struct xmethod_worker *worker, - struct value *obj, struct value **args, int nargs, - struct value **result) + struct xmethod_worker *worker, + struct value *obj, struct value **args, int nargs, + struct value **result) { int i; struct cleanup *cleanups; @@ -516,12 +531,15 @@ gdbpy_invoke_xmethod (const struct extension_language_defn *extlang, } make_cleanup_py_decref (invoke_method); + // FIXME: unprotected call to check_typedef. See py-value.c for example. + // Needs TRY_CATCH. Here and below. obj_type = check_typedef (value_type (obj)); this_type = check_typedef (type_object_to_type (worker_data->this_type)); if (TYPE_CODE (obj_type) == TYPE_CODE_PTR) { struct type *this_ptr = lookup_pointer_type (this_type); + // FIXME: unprotected call to value_cast. See py-value.c for example. if (!types_equal (obj_type, this_ptr)) obj = value_cast (this_ptr, obj); } @@ -529,11 +547,13 @@ gdbpy_invoke_xmethod (const struct extension_language_defn *extlang, { struct type *this_ref = lookup_reference_type (this_type); + // FIXME: unprotected call to value_cast. See py-value.c for example. if (!types_equal (obj_type, this_ref)) obj = value_cast (this_ref, obj); } else { + // FIXME: unprotected call to value_cast. See py-value.c for example. if (!types_equal (obj_type, this_type)) obj = value_cast (this_type, obj); } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4 v18] Add xmethod documentation and NEWS entry 2014-05-25 23:00 ` Doug Evans @ 2014-05-28 6:10 ` Doug Evans 0 siblings, 0 replies; 4+ messages in thread From: Doug Evans @ 2014-05-28 6:10 UTC (permalink / raw) To: Siva Chandra; +Cc: gdb-patches Doug Evans <xdje42@gmail.com> writes: > Siva Chandra <sivachandra@google.com> writes: > [...] > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi > index 72d58ad..026790f 100644 > --- a/gdb/doc/python.texi > +++ b/gdb/doc/python.texi > @@ -2327,8 +2327,8 @@ existing matcher with the same name as @code{matcher} if > globally. > @end defun > > -@node Writing a Xmethod > -@subsubsection Writing a Xmethod > +@node Writing an Xmethod > +@subsubsection Writing an Xmethod > @cindex writing xmethods in Python > > Implementing xmethods in Python will require implementing xmethod > @@ -2372,6 +2372,11 @@ class MyClassMatcher(gdb.xmethod.XMethodMatcher): > def match(self, class_type, method_name): > if class_type.tag != 'MyClass': > return None > + # FIXME: User code shouldn't have to check the enabled flag. > + # py-xmethods.c checks it, so can this check just be removed? > + # If user code wants to add this enabled check, e.g., should a perf > + # concern arise, then ok; but we shouldn't encourage it as the default > + # mode of writing xmethods. > if method_name == 'geta' and self.methods[0].enabled: > return MyClassWorker_geta() > elif method_name == 'operator+' and self.methods[1].enabled: > @@ -2386,6 +2391,7 @@ a worker object of type @code{MyClassWorker_geta} for the @code{geta} > method, and a worker object of type @code{MyClassWorker_plus} for the > @code{operator+} method. Also, a worker object is returned only if the > corresponding entry in the @code{methods} attribute is enabled. > +@c FIXME: See above: User code shouldn't check enabled flag, gdb should. > > The implementation of the worker classes returned by the matcher above > is as follows: Ok, I was wrong here. What's going on here is equivalent to what python/lib/gdb/printing.y:RegexpCollectionPrettyPrinter is doing here: # Iterate over table of type regexps to determine # if a printer is registered for that type. # Return an instantiation of the printer if found. for printer in self.subprinters: if printer.enabled and printer.compiled_re.search(typename): return printer.gen_printer(val) However, note that in the pretty-printer case the result is obtained by invoking a method on the subprinter: return printer.gen_printer(val) whereas in the MyClassMatcher case all self.methods is used for is the enabled flag: if method_name == 'geta' and self.methods[0].enabled: return MyClassWorker_geta() elif method_name == 'operator+' and self.methods[1].enabled: return MyClassWorker_plus() else: return None It would be cleaner to have an example where this code looked something like the pretty-printer case: for method in self.methods: if method.enabled and method.name == method_name return method.gen_worker() return None Maybe something like: class MyClassMatcher(gdb.xmethod.XMethodMatcher): class MyClassMethod(gdb.xmethod.XMethod): def __init(self, name, worker): gdb.xmethod.XMethod.__init__(self, name) self.worker = worker def __init__(self): gdb.xmethod.XMethodMatcher.__init__(self, 'MyMatcher') # List of methods 'managed' by this matcher self.methods = [MyClassMethod('geta', MyClassWorker_geta()), MyClassXMethod('operator+', MyClassWorker_plus())] def __call__(self, class_type, method_name): if class_type.tag != 'MyClass': return None for method in self.methods: if method.enabled and method.name == method_name return method.worker return None ? ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-05-28 6:10 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-05-23 21:55 [PATCH 1/4 v18] Add xmethod documentation and NEWS entry Siva Chandra 2014-05-24 7:13 ` Eli Zaretskii 2014-05-25 23:00 ` Doug Evans 2014-05-28 6:10 ` Doug Evans
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).