Re: [PATCH 2/4 v15] Add xmethod interface to the extension language API

[Doug Evans <dje at google dot com>]

Siva Chandra writes:
 > The attached patch addresses Doug's comments from his last review.
 > Also, it has been updated per the IRC discussions around
 > "lval_xcallable". It was suggested that I grep for lval_internalvar
 > and add similar logic for lval_xcallable. I have done this, but only
 > in those places I felt it is meaningful.

Good idea. :)
Not all occurrences of lval_internalvar would be reached for TYPE_CODE_XMETHOD
I think.

I just have nits below.

 > Posting of the previous version:
 > https://sourceware.org/ml/gdb-patches/2014-04/msg00585.html
 > 
 > ChangeLog:
 > 2014-05-17  Siva Chandra Reddy  <sivachandra@google.com>
 > 
 >         * defs.h (enum lval_type): New enumerator "lval_xcallable".
 >         * extension-priv.h (struct extension_language_ops): Add the
 >         xmethod interface.
 >         * extension.c (new_xmethod_worker, clone_xmethod_worker,
 >         get_matching_xmethod_workers, get_xmethod_argtypes,
 >         invoke_xmethod, free_xmethod_worker,
 >         free_xmethod_worker_vec): New functions.
 >         * extension.h: #include "common/vec.h".
 >         New function declarations.
 >         (struct xmethod_worker): New struct.
 >         (VEC (xmethod_worker_ptr)): New vector type.
 >         (xmethod_worker_ptr): New typedef.
 >         (xmethod_worker_vec): Likewise.
 >         * gdbtypes.c (gdbtypes_post_init): Initialize "xmethod" field of
 >         builtin_type.
 >         * gdbtypes.h (enum type_code): New enumerator TYPE_CODE_XMETHOD.
 >         (struct builtin_type): New field "xmethod".
 >         * valarith.c (value_ptradd): Assert that the value argument is not
 >         lval_xcallable.
 >         * valops.c (value_must_coerce_to_target): Return 0 for
 >         lval_xcallable values.
 >         * value.c (struct value): New field XM_WORKER in the field
 >         LOCATION.
 >         (value_address, value_raw_address): Return 0 for lval_xcallable
 >         values.
 >         (set_value_address): Assert that the value is not an
 >         lval_xcallable.
 >         (value_free): Free the associated xmethod worker when freeing
 >         lval_xcallable values.
 >         (set_value_component_location): Assert that the WHOLE value is not
 >         lval_xcallable.
 >         (value_of_xmethod, call_xmethod, free_xmethod_value): New
 >         functions.
 >         * value.h: Declare "struct xmethod_worker".
 >         Declare new functions value_of_xmethod, call_xmethod
 >         and free_xmethod_value.

	Declare new functions value_of_xmethod, call_xmethod
	and free_xmethod_value.

->

	(value_of_xmethod): Declare.
	(call_xmethod,free_xmethod_value): Declare.

 > diff --git a/gdb/defs.h b/gdb/defs.h
 > index 47da43a..8f73353 100644
 > --- a/gdb/defs.h
 > +++ b/gdb/defs.h
 > @@ -388,6 +388,8 @@ enum lval_type
 >      lval_register,
 >      /* * In a gdb internal variable.  */
 >      lval_internalvar,
 > +    /* * Value encapsulates a callable defined in an extension languages.  */

extension language

 > +    lval_xcallable,
 >      /* * Part of a gdb internal variable (structure field).  */
 >      lval_internalvar_component,
 >      /* * Value's bits are fetched and stored using functions provided
 > diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
 > index 9e63a9c..0beedad 100644
 > --- a/gdb/extension-priv.h
 > +++ b/gdb/extension-priv.h
 > @@ -256,6 +256,54 @@ struct extension_language_ops
 >       changed or an error occurs no further languages are called.  */
 >    enum ext_lang_rc (*before_prompt) (const struct extension_language_defn *,
 >  				     const char *current_gdb_prompt);
 > +
 > +  /* xmethod support:
 > +     clone_xmethod_worker_data, free_xmethod_worker_data,
 > +     get_matching_xmethod_workers, get_xmethod_arg_types,
 > +     invoke_xmethod.
 > +     These methods are optional and may be NULL, but if one of them is
 > +     implemented then they all must be.  */
 > +
 > +  /* Clone DATA and return a new but identical xmethod worker data
 > +     object for this extension language.  */
 > +  void * (*clone_xmethod_worker_data)
 > +    (const struct extension_language_defn *extlang, void *data);
 > +
 > +  /* Free the DATA object of this extension language.  */
 > +  void (*free_xmethod_worker_data)
 > +    (const struct extension_language_defn *extlang, void *data);
 > +
 > +  /* Return a vector of matching xmethod workers defined in this
 > +     extension language.  The workers service methods with name
 > +     METHOD_NAME on objects of type OBJ_TYPE.  The vector is returned
 > +     in DM_VEC.  */
 > +  enum ext_lang_rc (*get_matching_xmethod_workers)
 > +    (const struct extension_language_defn *extlang,
 > +     struct type *obj_type,
 > +     const char *method_name,
 > +     xmethod_worker_vec **dm_vec);
 > +
 > +  /* Given a WORKER servicing a particular method, return the types
 > +     of the arguments the method takes.  The number of arguments is
 > +     returned in NARGS, and their types are returned in the array
 > +     ARGTYPES.  */
 > +  enum ext_lang_rc (*get_xmethod_arg_types)
 > +    (const struct extension_language_defn *extlang,
 > +     struct xmethod_worker *worker,
 > +     int *nargs,
 > +     struct type ***arg_types);
 > +
 > +  /* Invoke the xmethod serviced by WORKER.  The method is invoked
 > +     on OBJECT with arguments in the array ARGS.  NARGS is the length of
 > +     this array.  The value returned by the method is returned in
 > +     RESULT.  */
 > +  enum ext_lang_rc (*invoke_xmethod)
 > +    (const struct extension_language_defn *extlang,
 > +     struct xmethod_worker *worker,
 > +     struct value *object,
 > +     struct value **args,
 > +     int nargs,
 > +     struct value **result);
 >  };
 >  
 >  /* State necessary to restore a signal handler to its previous value.  */
 > diff --git a/gdb/extension.c b/gdb/extension.c
 > index 1146cc7..b972485 100644
 > --- a/gdb/extension.c
 > +++ b/gdb/extension.c
 > @@ -834,6 +834,170 @@ check_quit_flag (void)
 >    return result;
 >  }
 >  
 > +/* xmethod support.  */
 > +
 > +/* The xmethod API routines do not have "ext_lang" in the name because
 > +   the name "xmethod" implies that this routine deals with extension
 > +   languages.  Plus some of the methods take a xmethod_foo * "self/this"
 > +   arg, not an extension_language_defn * arg.  */
 > +
 > +/* Returns a new xmethod_worker with EXTLANG and DATA.  Space for the
 > +   result is allocated using malloc.  Caller must free.  */
 > +
 > +struct xmethod_worker *
 > +new_xmethod_worker (const struct extension_language_defn *extlang, void *data)
 > +{
 > +  struct xmethod_worker *worker = XCNEW (struct xmethod_worker);
 > +
 > +  worker->extlang = extlang;
 > +  worker->data = data;
 > +  worker->value = NULL;
 > +
 > +  return worker;
 > +}
 > +
 > +/* Clones WORKER and returns a new but identical worker.
 > +   The function get_matching_xmethod_workers (see below), returns a
 > +   vector of matching workers.  If a particular worker is selected by GDB
 > +   to invoke a method, then this function can help in cloning the
 > +   selected worker and freeing up the vector via a cleanup.
 > +
 > +   Space for the result is allocated using malloc.  Caller must free.  */

More than one call to xfree is required, right?
And the caller shouldn't need to know the details.
How about instead say:
Space for the result must be freed with free_xmethod_worker.

 > +
 > +struct xmethod_worker *
 > +clone_xmethod_worker (struct xmethod_worker *worker)
 > +{
 > +  struct xmethod_worker *new_worker;
 > +  const struct extension_language_defn *extlang = worker->extlang;
 > +
 > +  gdb_assert (extlang->ops->clone_xmethod_worker_data != NULL);
 > +
 > +  new_worker = new_xmethod_worker
 > +    (extlang,
 > +     extlang->ops->clone_xmethod_worker_data (extlang, worker->data));
 > +
 > +  return new_worker;
 > +}
 > +
 > +/* If a method with name METHOD_NAME is to be invoked on an object of type
 > +   TYPE, then all entension languages are searched for implementations of
 > +   methods with name METHOD.  All matches found are returned as a vector
 > +   of 'xmethod_worker_ptr' objects.  If no matching methods are
 > +   found, NULL is returned.  */
 > +
 > +VEC (xmethod_worker_ptr) *
 > +get_matching_xmethod_workers (struct type *type, const char *method_name)
 > +{
 > +  VEC (xmethod_worker_ptr) *workers = NULL;
 > +  int i;
 > +  const struct extension_language_defn *extlang;
 > +
 > +  ALL_ENABLED_EXTENSION_LANGUAGES (i, extlang)
 > +    {
 > +      VEC (xmethod_worker_ptr) *lang_workers, *new_vec;
 > +      enum ext_lang_rc rc;
 > +
 > +      /* If an extension language does not support xmethods, ignore
 > +	 it.  */
 > +      if (extlang->ops->get_matching_xmethod_workers == NULL)
 > +	continue;
 > +
 > +      rc = extlang->ops->get_matching_xmethod_workers (extlang,
 > +						       type, method_name,
 > +						       &lang_workers);
 > +      if (rc == EXT_LANG_RC_ERROR)
 > +	{
 > +	  free_xmethod_worker_vec (workers);
 > +	  error (_("Error while looking for matching xmethod workers "
 > +		   "defined in %s."), extlang->capitalized_name);
 > +	}
 > +
 > +      new_vec = VEC_merge (xmethod_worker_ptr, workers, lang_workers);
 > +      /* Free only the vectors and not the elements as NEW_VEC still
 > +	 contains them.  */
 > +      VEC_free (xmethod_worker_ptr, workers);
 > +      VEC_free (xmethod_worker_ptr, lang_workers);
 > +      workers = new_vec;
 > +    }
 > +
 > +  return workers;
 > +}
 > +
 > +/* Return the arg types of the xmethod encapsulated in WORKER.
 > +   An array of arg types is returned.  The length of the array is returned in
 > +   NARGS.  The type of the 'this' object is returned as the first element of
 > +   array.  */
 > +
 > +struct type **
 > +get_xmethod_arg_types (struct xmethod_worker *worker, int *nargs)
 > +{
 > +  enum ext_lang_rc rc;
 > +  struct type **type_array = NULL;
 > +  const struct extension_language_defn *extlang = worker->extlang;
 > +
 > +  gdb_assert (extlang->ops->get_xmethod_arg_types != NULL);
 > +
 > +  rc = extlang->ops->get_xmethod_arg_types (extlang, worker, nargs,
 > +					    &type_array);
 > +  if (rc == EXT_LANG_RC_ERROR)
 > +    {
 > +      error (_("Error while looking for arg types of a xmethod worker "
 > +	       "defined in %s."), extlang->capitalized_name);
 > +    }
 > +
 > +  return type_array;
 > +}
 > +
 > +/* Invokes the xmethod encapsulated in WORKER and returns the result.
 > +   The method is invoked on OBJ with arguments in the ARGS array.  NARGS is
 > +   the length of the this array.  */
 > +
 > +struct value *
 > +invoke_xmethod (struct xmethod_worker *worker, struct value *obj,
 > +		     struct value **args, int nargs)
 > +{
 > +  struct value *result = NULL;
 > +  enum ext_lang_rc rc;
 > +
 > +  gdb_assert (worker->extlang->ops->invoke_xmethod != NULL);
 > +
 > +  rc = worker->extlang->ops->invoke_xmethod (worker->extlang, worker,
 > +					     obj, args, nargs, &result);
 > +  if (rc == EXT_LANG_RC_ERROR)
 > +    {
 > +      error (_("Error while invoking a xmethod defined in %s"),
 > +	     worker->extlang->capitalized_name);
 > +    }
 > +
 > +  return result;
 > +}
 > +
 > +/* Frees the xmethod worker WORKER.  */
 > +
 > +void
 > +free_xmethod_worker (struct xmethod_worker *worker)
 > +{
 > +  gdb_assert (worker->extlang->ops->free_xmethod_worker_data != NULL);
 > +  worker->extlang->ops->free_xmethod_worker_data (worker->extlang,
 > +						  worker->data);
 > +  xfree (worker);
 > +}
 > +
 > +/* Frees a vector of xmethod_workers VEC.  */
 > +
 > +void
 > +free_xmethod_worker_vec (void *vec)
 > +{
 > +  int i;
 > +  struct xmethod_worker *worker;
 > +  VEC (xmethod_worker_ptr) *v = (VEC (xmethod_worker_ptr) *) vec;
 > +
 > +  for (i = 0; VEC_iterate (xmethod_worker_ptr, v, i, worker); i++)
 > +    free_xmethod_worker (worker);
 > +
 > +  VEC_free (xmethod_worker_ptr, v);
 > +}
 > +
 >  /* Called via an observer before gdb prints its prompt.
 >     Iterate over the extension languages giving them a chance to
 >     change the prompt.  The first one to change the prompt wins,
 > diff --git a/gdb/extension.h b/gdb/extension.h
 > index 61dc81b..c08d98d 100644
 > --- a/gdb/extension.h
 > +++ b/gdb/extension.h
 > @@ -21,6 +21,7 @@
 >  #define EXTENSION_H
 >  
 >  #include "mi/mi-cmds.h" /* For PRINT_NO_VALUES, etc.  */
 > +#include "common/vec.h"
 >  
 >  struct breakpoint;
 >  struct command_line;
 > @@ -138,6 +139,26 @@ struct ext_lang_type_printers
 >    /* Type-printers from Python.  */
 >    void *py_type_printers;
 >  };
 > +
 > +/* A type which holds its extension language specific xmethod worker data.  */
 > +
 > +struct xmethod_worker
 > +{
 > +  /* The language the xmethod worker is implemented in.  */
 > +  const struct extension_language_defn *extlang;
 > +
 > +  /* The extension language specific data for this xmethod worker.  */
 > +  void *data;
 > +
 > +  /* The TYPE_CODE_XMETHOD value corresponding to this worker.
 > +     Always use value_of_xmethod to access it.  */
 > +  struct value *value;
 > +};
 > +
 > +typedef struct xmethod_worker *xmethod_worker_ptr;
 > +DEF_VEC_P (xmethod_worker_ptr);
 > +typedef VEC (xmethod_worker_ptr) xmethod_worker_vec;
 > +
 >  
 >  /* The interface for gdb's own extension(/scripting) language.  */
 >  extern const struct extension_language_defn extension_language_gdb;
 > @@ -212,4 +233,22 @@ extern const struct extension_language_defn *get_breakpoint_cond_ext_lang
 >  
 >  extern int breakpoint_ext_lang_cond_says_stop (struct breakpoint *);
 >  
 > +extern struct value *invoke_xmethod (struct xmethod_worker *,
 > +				     struct value *,
 > +				     struct value **, int nargs);
 > +
 > +extern struct xmethod_worker *clone_xmethod_worker (struct xmethod_worker *);
 > +
 > +extern struct xmethod_worker *new_xmethod_worker
 > +  (const struct extension_language_defn *extlang, void *data);
 > +
 > +extern void free_xmethod_worker (struct xmethod_worker *);
 > +
 > +extern void free_xmethod_worker_vec (void *vec);
 > +
 > +extern xmethod_worker_vec *get_matching_xmethod_workers
 > +  (struct type *, const char *);
 > +
 > +extern struct type **get_xmethod_arg_types (struct xmethod_worker *, int *);
 > +
 >  #endif /* EXTENSION_H */
 > diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
 > index 8e6631a..56bcf59 100644
 > --- a/gdb/gdbtypes.c
 > +++ b/gdb/gdbtypes.c
 > @@ -4387,6 +4387,10 @@ gdbtypes_post_init (struct gdbarch *gdbarch)
 >      = arch_type (gdbarch, TYPE_CODE_INTERNAL_FUNCTION, 0,
 >  		 "<internal function>");
 >  
 > +  /* This type represents an xmethod.  */
 > +  builtin_type->xmethod
 > +    = arch_type (gdbarch, TYPE_CODE_XMETHOD, 0, "<xmethod>");
 > +
 >    return builtin_type;
 >  }
 >  
 > diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
 > index 86b1d62..bb6352d 100644
 > --- a/gdb/gdbtypes.h
 > +++ b/gdb/gdbtypes.h
 > @@ -179,7 +179,10 @@ enum type_code
 >      TYPE_CODE_MODULE,		/**< Fortran module.  */
 >  
 >      /* * Internal function type.  */
 > -    TYPE_CODE_INTERNAL_FUNCTION
 > +    TYPE_CODE_INTERNAL_FUNCTION,
 > +
 > +    /* * Methods implemented in extension languages.  */
 > +    TYPE_CODE_XMETHOD
 >    };
 >  
 >  /* * For now allow source to use TYPE_CODE_CLASS for C++ classes, as
 > @@ -1468,6 +1471,9 @@ struct builtin_type
 >    /* * This type is used to represent a GDB internal function.  */
 >  
 >    struct type *internal_fn;
 > +
 > +  /* * This type is used to represent an xmethod.  */
 > +  struct type *xmethod;
 >  };
 >  
 >  /* * Return the type table for the specified architecture.  */
 > diff --git a/gdb/valarith.c b/gdb/valarith.c
 > index 8e863e3..3a53961 100644
 > --- a/gdb/valarith.c
 > +++ b/gdb/valarith.c
 > @@ -89,6 +89,8 @@ value_ptradd (struct value *arg1, LONGEST arg2)
 >    LONGEST sz;
 >    struct value *result;
 >  
 > +  gdb_assert (VALUE_LVAL (arg1) != lval_xcallable);
 > +

Heh.  I think I understand how this addition got in,
but I think we can skip it.  [My concern is future readers
looking at this and then wondering why value_ptrdiff, et.al.
don't have similar asserts.]
lval_internalvar is used for both internal variables and internal functions,
but this function will only ever get called for internal variables.
Later in this function we do:

  if (VALUE_LVAL (result) != lval_internalvar)
    set_value_component_location (result, arg1);

but I doubt we ever get into this function for internal functions.
I think that's there just for internal variables.

 >    arg1 = coerce_array (arg1);
 >    valptrtype = check_typedef (value_type (arg1));
 >    sz = find_size_for_pointer_math (valptrtype);
 > diff --git a/gdb/valops.c b/gdb/valops.c
 > index ff25f1a..e915e34 100644
 > --- a/gdb/valops.c
 > +++ b/gdb/valops.c
 > @@ -1372,7 +1372,8 @@ value_must_coerce_to_target (struct value *val)
 >  
 >    /* The only lval kinds which do not live in target memory.  */
 >    if (VALUE_LVAL (val) != not_lval
 > -      && VALUE_LVAL (val) != lval_internalvar)
 > +      && VALUE_LVAL (val) != lval_internalvar
 > +      && VALUE_LVAL (val) != lval_xcallable)
 >      return 0;
 >  
 >    valtype = check_typedef (value_type (val));
 > diff --git a/gdb/value.c b/gdb/value.c
 > index d125a09..4180186 100644
 > --- a/gdb/value.c
 > +++ b/gdb/value.c
 > @@ -230,6 +230,9 @@ struct value
 >      /* Pointer to internal variable.  */
 >      struct internalvar *internalvar;
 >  
 > +    /* Pointer to xmethod worker.  */
 > +    struct xmethod_worker *xm_worker;
 > +
 >      /* If lval == lval_computed, this is a set of function pointers
 >         to use to access and describe the value, and a closure pointer
 >         for them to use.  */
 > @@ -1340,7 +1343,8 @@ CORE_ADDR
 >  value_address (const struct value *value)
 >  {
 >    if (value->lval == lval_internalvar
 > -      || value->lval == lval_internalvar_component)
 > +      || value->lval == lval_internalvar_component
 > +      || value->lval == lval_xcallable)
 >      return 0;
 >    if (value->parent != NULL)
 >      return value_address (value->parent) + value->offset;
 > @@ -1352,7 +1356,8 @@ CORE_ADDR
 >  value_raw_address (struct value *value)
 >  {
 >    if (value->lval == lval_internalvar
 > -      || value->lval == lval_internalvar_component)
 > +      || value->lval == lval_internalvar_component
 > +      || value->lval == lval_xcallable)
 >      return 0;
 >    return value->location.address;
 >  }
 > @@ -1361,7 +1366,8 @@ void
 >  set_value_address (struct value *value, CORE_ADDR addr)
 >  {
 >    gdb_assert (value->lval != lval_internalvar
 > -	      && value->lval != lval_internalvar_component);
 > +	      && value->lval != lval_internalvar_component
 > +	      && value->lval != lval_xcallable);
 >    value->location.address = addr;
 >  }
 >  
 > @@ -1433,6 +1439,8 @@ value_free (struct value *val)
 >  	  if (funcs->free_closure)
 >  	    funcs->free_closure (val);
 >  	}
 > +      else if (VALUE_LVAL (val) == lval_xcallable)
 > +	  free_xmethod_worker (val->location.xm_worker);
 >  
 >        xfree (val->contents);
 >        VEC_free (range_s, val->unavailable);
 > @@ -1623,6 +1631,8 @@ void
 >  set_value_component_location (struct value *component,
 >  			      const struct value *whole)
 >  {
 > +  gdb_assert (whole->lval != lval_xcallable);
 > +
 >    if (whole->lval == lval_internalvar)
 >      VALUE_LVAL (component) = lval_internalvar_component;
 >    else
 > @@ -2456,6 +2466,55 @@ show_convenience (char *ignore, int from_tty)
 >      }
 >  }
 >  
 > +/* Return the TYPE_CODE_XMETHOD value corresponding to WORKER.  When done with
 > +   the value and the WORKER, free it using free_xmethod_value.  Freeing the
 > +   value frees the WORKER as well.  */
 > +
 > +struct value *
 > +value_of_xmethod (struct xmethod_worker *worker)
 > +{
 > +  if (worker->value == NULL)
 > +    {
 > +      struct value *v;
 > +
 > +      v = allocate_value (builtin_type (target_gdbarch ())->xmethod);
 > +      v->lval = lval_xcallable;
 > +      v->location.xm_worker = worker;
 > +      v->modifiable = 0;
 > +      worker->value = v;
 > +    }
 > +
 > +  return worker->value;
 > +}
 > +
 > +/* Call the xmethod corresponding to the TYPE_CODE_XMETHOD value METHOD.  */
 > +
 > +struct value *
 > +call_xmethod (struct value *method, int argc, struct value **argv)
 > +{
 > +  gdb_assert (TYPE_CODE (value_type (method)) == TYPE_CODE_XMETHOD
 > +	      && method->lval == lval_xcallable && argc > 0);
 > +
 > +  return invoke_xmethod (method->location.xm_worker,
 > +			 argv[0], argv + 1, argc - 1);
 > +}
 > +
 > +/* Free the TYPE_CODE_XMETHOD value XM_VALUE.  Releases the value from the
 > +   value chain and frees it.  The xmethod worker object corresponding to
 > +   this value is also freed.  */
 > +
 > +void
 > +free_xmethod_value (void *xm_value)

Not sure about this function, but let's leave it as is for now.

 > +{
 > +  struct value *v = xm_value;
 > +
 > +  gdb_assert (TYPE_CODE (value_type (v)) == TYPE_CODE_XMETHOD
 > +	      && v->lval == lval_xcallable);
 > +
 > +  release_value (v);
 > +  value_free (v);
 > +}
 > +
 >  /* Extract a value as a C number (either long or double).
 >     Knows how to convert fixed values to double, or
 >     floating values to long.
 > diff --git a/gdb/value.h b/gdb/value.h
 > index 144e182..927bbad 100644
 > --- a/gdb/value.h
 > +++ b/gdb/value.h
 > @@ -31,6 +31,7 @@ struct type;
 >  struct ui_file;
 >  struct language_defn;
 >  struct value_print_options;
 > +struct xmethod_worker;
 >  
 >  /* The structure which defines the type of a value.  It should never
 >     be possible for a program lval value to survive over a call to the
 > @@ -857,6 +858,8 @@ extern int compile_internalvar_to_ax (struct internalvar *var,
 >  
 >  extern struct internalvar *lookup_internalvar (const char *name);
 >  
 > +extern struct value *value_of_xmethod (struct xmethod_worker *);
 > +
 >  extern int value_equal (struct value *arg1, struct value *arg2);
 >  
 >  extern int value_equal_contents (struct value *arg1, struct value *arg2);
 > @@ -1011,4 +1014,9 @@ struct value *call_internal_function (struct gdbarch *gdbarch,
 >  
 >  char *value_internal_function_name (struct value *);
 >  
 > +struct value *call_xmethod (struct value *function,
 > +			    int argc, struct value **argv);
 > +
 > +extern void free_xmethod_value (void *);
 > +
 >  #endif /* !defined (VALUE_H) */

Nit: Keep the xmethod decls together in value.h.