This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/4 v16] Add support for lookup, overload resolution and invocation of xmethods of C++ classes
- From: Doug Evans <dje at google dot com>
- To: Siva Chandra <sivachandra at google dot com>
- Cc: gdb-patches <gdb-patches at sourceware dot org>
- Date: Tue, 20 May 2014 17:13:00 -0700
- Subject: Re: [PATCH 3/4 v16] Add support for lookup, overload resolution and invocation of xmethods of C++ classes
- Authentication-results: sourceware.org; auth=none
- References: <CAGyQ6gzG-WK_nfDY5Z_akroBCw3QESMqPDRE5NS91nM5pvz3-A at mail dot gmail dot com>
Siva Chandra writes:
> The attached patch addresses Doug's comments from his last round of
> review and changes the var names to xmethod (from debug_method).
>
> On Mon, May 19, 2014 at 5:25 PM, Doug Evans <dje@google.com> wrote:
> > Heh. I'll bet the arch/language arguments are in call_internal_function
> > because of python (since that's what is passed to ensure_python_env).
> > So now I'm wondering where the python invoke_xmethod support gets its
> > gdbarch from. Have to check the python patch.
>
> In the python patch, I have passed get_current_arch () and
> current_language to ensure_python_env. Is this not good enough?
>
> ChangeLog
> 2014-05-20 Siva Chandra Reddy <sivachandra@google.com>
>
> * eval.c (evaluate_subexp_standard): Call the xmethod if the
> best match method returned by find_overload_match is an xmethod.
> * valarith.c (value_x_binop, value_x_unop): Call the xmethod if
> the best matching operator returned by find_overload_match is an
> xmethod.
> * valops.c: #include "extension.h".
> (find_method_list): Add "fn_list" and "xm_worker_vec" arguments.
> Return void. The list of matching source methods is returned in
> "fn_list" and a vector of matching debug method workers is
> returned in "xm_worker_vec". Update all callers.
> (value_find_oload_method_list): Likewise.
> (find_oload_champ): Add "xm_worker_vec" parameter. If it is
> non-NULL, then the index of the best matching method in this
> vector is returned. Update all callers.
> (find_overload_match): Include xmethods while performing overload
> resolution.
> (value_has_indirect_dynamic_type, cast_args_to_param_types,
> equal_param_types_p, derived_hides_base_method): New functions.
For good or bad, the last entry violates The Rules.
The parenthesized list of symbols cannot span multiple lines.
Yeah, The Rules are that pedantic.
This is ok, for example:
(value_has_indirect_dynamic_type, cast_args_to_param_types)
(equal_param_types_p, derived_hides_base_method): New functions.
> diff --git a/gdb/eval.c b/gdb/eval.c
> index 14bf6f7..ce516d1 100644
> --- a/gdb/eval.c
> +++ b/gdb/eval.c
> @@ -1765,11 +1765,25 @@ evaluate_subexp_standard (struct type *expect_type,
> error (_("Expression of type other than "
> "\"Function returning ...\" used as function"));
> }
> - if (TYPE_CODE (value_type (argvec[0])) == TYPE_CODE_INTERNAL_FUNCTION)
> - return call_internal_function (exp->gdbarch, exp->language_defn,
> - argvec[0], nargs, argvec + 1);
> + switch (TYPE_CODE (value_type (argvec[0])))
> + {
> + case TYPE_CODE_INTERNAL_FUNCTION:
> + return call_internal_function (exp->gdbarch, exp->language_defn,
> + argvec[0], nargs, argvec + 1);
> + case TYPE_CODE_XMETHOD:
> + {
> + struct value *retval;
> + struct cleanup *xm_cleanup;
> +
> + xm_cleanup = make_cleanup (free_xmethod_value, argvec[0]);
> + retval = call_xmethod (argvec[0], nargs, argvec + 1);
> + do_cleanups (xm_cleanup);
The fact that the other cases don't need cleanups
makes me think xm_cleanup isn't needed here.
values get garbage collected as a bunch after evaluating the entire
expression (or before executing the next command), and presumably xmethod
values will too. Let's take out the cleanup for now.
Ditto for the xm_cleanups below in value_x_binop, value_x_unop.
[I'll confirm it's ok when I do one last pass over the entire patch set.
No need to resend 1,2,4 just yet. Let's get #3 (mostly) finished first.]
>
> - return call_function_by_hand (argvec[0], nargs, argvec + 1);
> + return retval;
> + }
> + default:
> + return call_function_by_hand (argvec[0], nargs, argvec + 1);
> + }
> /* pai: FIXME save value from call_function_by_hand, then adjust
> pc by adjust_fn_pc if +ve. */
>
> diff --git a/gdb/valarith.c b/gdb/valarith.c
> index 3a53961..511996e 100644
> --- a/gdb/valarith.c
> +++ b/gdb/valarith.c
> @@ -491,8 +491,21 @@ value_x_binop (struct value *arg1, struct value *arg2, enum exp_opcode op,
> = TYPE_TARGET_TYPE (check_typedef (value_type (argvec[0])));
> return value_zero (return_type, VALUE_LVAL (arg1));
> }
> - return call_function_by_hand (argvec[0], 2 - static_memfuncp,
> - argvec + 1);
> +
> + if (TYPE_CODE (value_type (argvec[0])) == TYPE_CODE_XMETHOD)
> + {
> + struct value *retval;
> + struct cleanup *xm_cleanup;
> +
> + xm_cleanup = make_cleanup (free_xmethod_value, argvec[0]);
I think an assert that static_memfuncp == 0 would be good here.
And a comment that static methods aren't supported. yet.
Ditto for value_x_unop.
> + retval = call_xmethod (argvec[0], 2, argvec + 1);
> + do_cleanups (xm_cleanup);
> +
> + return retval;
> + }
> + else
> + return call_function_by_hand (argvec[0], 2 - static_memfuncp,
> + argvec + 1);
> }
> throw_error (NOT_FOUND_ERROR,
> _("member function %s not found"), tstr);
> @@ -595,7 +608,19 @@ value_x_unop (struct value *arg1, enum exp_opcode op, enum noside noside)
> = TYPE_TARGET_TYPE (check_typedef (value_type (argvec[0])));
> return value_zero (return_type, VALUE_LVAL (arg1));
> }
> - return call_function_by_hand (argvec[0], nargs, argvec + 1);
> + if (TYPE_CODE (value_type (argvec[0])) == TYPE_CODE_XMETHOD)
> + {
> + struct value *retval;
> + struct cleanup *xm_cleanup;
> +
> + xm_cleanup = make_cleanup (free_xmethod_value, argvec[0]);
> + retval = call_xmethod (argvec[0], 1, argvec + 1);
> + do_cleanups (xm_cleanup);
> +
> + return retval;
> + }
> + else
> + return call_function_by_hand (argvec[0], nargs, argvec + 1);
> }
> throw_error (NOT_FOUND_ERROR,
> _("member function %s not found"), tstr);
> diff --git a/gdb/valops.c b/gdb/valops.c
> index e915e34..0539f51 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -42,6 +42,7 @@
> #include "observer.h"
> #include "objfiles.h"
> #include "exceptions.h"
> +#include "extension.h"
>
> extern unsigned int overload_debug;
> /* Local functions. */
> @@ -70,8 +71,8 @@ int find_oload_champ_namespace_loop (struct value **, int,
> const int no_adl);
>
> static int find_oload_champ (struct value **, int, int,
> - struct fn_field *, struct symbol **,
> - struct badness_vector **);
> + struct fn_field *, VEC (xmethod_worker_ptr) *,
> + struct symbol **, struct badness_vector **);
>
> static int oload_method_static_p (struct fn_field *, int);
>
> @@ -98,9 +99,10 @@ static CORE_ADDR allocate_space_in_inferior (int);
>
> static struct value *cast_into_complex (struct type *, struct value *);
>
> -static struct fn_field *find_method_list (struct value **, const char *,
> - int, struct type *, int *,
> - struct type **, int *);
> +static void find_method_list (struct value **, const char *,
> + int, struct type *, struct fn_field **, int *,
> + VEC (xmethod_worker_ptr) **,
> + struct type **, int *);
>
> void _initialize_valops (void);
>
> @@ -2256,53 +2258,83 @@ value_struct_elt_bitpos (struct value **argp, int bitpos, struct type *ftype,
> }
>
> /* Search through the methods of an object (and its bases) to find a
> - specified method. Return the pointer to the fn_field list of
> - overloaded instances.
> + specified method. Return the pointer to the fn_field list FN_LIST of
> + overloaded instances defined in the source language. If available
> + and matching, a vector of matching xmethods defined in extension
> + languages are also returned in XM_WORKER_VEC
>
> Helper function for value_find_oload_list.
> ARGP is a pointer to a pointer to a value (the object).
> METHOD is a string containing the method name.
> OFFSET is the offset within the value.
> TYPE is the assumed type of the object.
> + FN_LIST The pointer to matching overloaded instances defined in
Stick with the style of the surrounding comments, don't invent.
E.g.,
FN_LIST is ...
> + source language.
> NUM_FNS is the number of overloaded instances.
> + XM_WORKER_VEC The vector of matching xmethod workers.
Ditto.
XM_WORKER_VEC is ...
> BASETYPE is set to the actual type of the subobject where the
> method is found.
> BOFFSET is the offset of the base subobject where the method is found. */
>
> -static struct fn_field *
> +static void
> find_method_list (struct value **argp, const char *method,
> - int offset, struct type *type, int *num_fns,
> + int offset, struct type *type,
> + struct fn_field **fn_list, int *num_fns,
> + VEC (xmethod_worker_ptr) **xm_worker_vec,
> struct type **basetype, int *boffset)
> {
> int i;
> - struct fn_field *f;
> - CHECK_TYPEDEF (type);
> + struct fn_field *f = NULL;
>
> - *num_fns = 0;
> + CHECK_TYPEDEF (type);
>
> - /* First check in object itself. */
> - for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; i--)
> + /* First check in object itself.
> + This function is called recursively to search through base classes.
> + If there is a source method match found at some stage, then we need not
> + look for source methods in consequent recursive calls. */
> + if (fn_list != NULL && (*fn_list) == NULL)
I can't see where fn_list would ever be NULL.
> {
> - /* pai: FIXME What about operators and type conversions? */
> - const char *fn_field_name = TYPE_FN_FIELDLIST_NAME (type, i);
> -
> - if (fn_field_name && (strcmp_iw (fn_field_name, method) == 0))
> + for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; i--)
> {
> - int len = TYPE_FN_FIELDLIST_LENGTH (type, i);
> - struct fn_field *f = TYPE_FN_FIELDLIST1 (type, i);
> + /* pai: FIXME What about operators and type conversions? */
> + const char *fn_field_name = TYPE_FN_FIELDLIST_NAME (type, i);
> +
> + if (fn_field_name && (strcmp_iw (fn_field_name, method) == 0))
> + {
> + int len = TYPE_FN_FIELDLIST_LENGTH (type, i);
> + f = TYPE_FN_FIELDLIST1 (type, i);
> + *fn_list = f;
>
> - *num_fns = len;
> - *basetype = type;
> - *boffset = offset;
> + *num_fns = len;
> + *basetype = type;
> + *boffset = offset;
>
> - /* Resolve any stub methods. */
> - check_stub_method_group (type, i);
> + /* Resolve any stub methods. */
> + check_stub_method_group (type, i);
>
> - return f;
> + break;
> + }
> }
> }
>
> - /* Not found in object, check in base subobjects. */
> + /* Unlike source methods, xmethods can be accumulated over successive
> + recursive calls. In other words, an xmethod named 'm' in a class
> + will not hide an xmethod named 'm' in its base class(es). */
IWBN to add a sentence here explaining *why* xmethod 'm' doesn't hide
'm' in base classes.
> + if (xm_worker_vec)
> + {
> + VEC (xmethod_worker_ptr) *worker_vec = NULL, *new_vec = NULL;
> +
> + worker_vec = get_matching_xmethod_workers (type, method);
> + new_vec = VEC_merge (xmethod_worker_ptr, *xm_worker_vec, worker_vec);
> +
> + VEC_free (xmethod_worker_ptr, *xm_worker_vec);
> + VEC_free (xmethod_worker_ptr, worker_vec);
> + *xm_worker_vec = new_vec;
> + }
> +
> + /* If source methods are not found in current class, look for them in the
> + base classes. We have to go through the base classes to gather extension
> + methods anyway. */
I would rephrase the second sentence to be:
We also have to go through the base classes to gather extension methods.
> for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--)
> {
> int base_offset;
> @@ -2319,28 +2351,35 @@ find_method_list (struct value **argp, const char *method,
> {
> base_offset = TYPE_BASECLASS_BITPOS (type, i) / 8;
> }
> - f = find_method_list (argp, method, base_offset + offset,
> - TYPE_BASECLASS (type, i), num_fns,
> - basetype, boffset);
> - if (f)
> - return f;
> +
> + find_method_list (argp, method, base_offset + offset,
> + TYPE_BASECLASS (type, i), fn_list, num_fns,
> + xm_worker_vec, basetype, boffset);
> }
> - return NULL;
> }
>
> -/* Return the list of overloaded methods of a specified name.
> +/* Return the list of overloaded methods of a specified name. The methods
> + could be those GDB finds in the binary, or xmethod. Methods found in
> + the binary are returned in FN_LIST, and xmethods are returned in
> + DM_WORKER_VEC.
>
> ARGP is a pointer to a pointer to a value (the object).
> METHOD is the method name.
> OFFSET is the offset within the value contents.
> + FN_LIST The pointer to matching overloaded instances defined in
Ditto. Follow the existing style, don't invent.
FN_LIST is ...
> + source language.
> NUM_FNS is the number of overloaded instances.
> + XM_WORKER_VEC The vector of matching xmethod workers defined in
XM_WORKER_VEC is ...
> + extension languages.
> BASETYPE is set to the type of the base subobject that defines the
> method.
> BOFFSET is the offset of the base subobject which defines the method. */
>
> -static struct fn_field *
> +static void
> value_find_oload_method_list (struct value **argp, const char *method,
> - int offset, int *num_fns,
> + int offset, struct fn_field **fn_list,
> + int *num_fns,
> + VEC (xmethod_worker_ptr) **xm_worker_vec,
> struct type **basetype, int *boffset)
> {
> struct type *t;
> @@ -2362,8 +2401,124 @@ value_find_oload_method_list (struct value **argp, const char *method,
> error (_("Attempt to extract a component of a "
> "value that is not a struct or union"));
>
> - return find_method_list (argp, method, 0, t, num_fns,
> - basetype, boffset);
> + /* Clear the lists. */
> + if (fn_list != NULL)
I think fn_list will never be NULL.
> + {
> + *fn_list = NULL;
> + *num_fns = 0;
> + }
> + if (xm_worker_vec != NULL)
Ditto, xm_worker_vec will never be NULL.
> + *xm_worker_vec = NULL;
> +
> + find_method_list (argp, method, 0, t, fn_list, num_fns, xm_worker_vec,
> + basetype, boffset);
> +}
> +
> +/* Return the dynamic type of OBJ. NULL is returned if OBJ does not have any
> + dynamic type. */
> +
> +static struct type *
> +value_has_indirect_dynamic_type (struct value *obj)
> +{
> + struct type *stype, *dtype, *dtype_ind;
> +
> + stype = check_typedef (TYPE_TARGET_TYPE (value_type (obj)));
TYPE_TARGET_TYPE can't be called on an arbitrary type.
Presumably OBJ can only be a pointer or a reference (right?).
Add an assert to verify that before calling TYPE_TARGET_TYPE.
> + dtype_ind = value_rtti_indirect_type (obj, NULL, NULL, NULL);
> + dtype = dtype_ind ? check_typedef (TYPE_TARGET_TYPE (dtype_ind)) : stype;
> +
> + if (class_types_same_p (stype, dtype))
> + return NULL;
> + else
> + return dtype_ind;
> +}
> +
> +/* Casts the arguments in the array ARGS to the types of the parameters of
> + the M-th method in FNS_PTR. The length of the array ARGS is given by
> + NARGS. This is a helper function for find_overload_match. */
> +
> +static void
> +cast_args_to_param_types (struct value **args, int nargs,
> + struct fn_field *fns_ptr, int m)
> +{
> + int i;
> +
> + gdb_assert (TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr, m)) == nargs);
> +
> + for (i = 1; i < nargs; i++)
> + {
> + struct type *param_type = TYPE_FN_FIELD_ARGS (fns_ptr, m)[i].type;
> + struct type *arg_type = check_typedef (value_type (args[i]));
> +
> + CHECK_TYPEDEF (param_type);
> + if (TYPE_CODE (param_type) == TYPE_CODE_REF
> + && TYPE_CODE (arg_type) != TYPE_CODE_REF)
> + param_type = TYPE_TARGET_TYPE (param_type);
> +
> + args[i] = value_cast (param_type, args[i]);
I'm not sure this is correct in all cases.
At the very least we can't let any exceptions value_cast may throw leak out.
Wrap the call to value_cast in a TRY_CATCH,
and if an exception is caught punt I guess (tell the caller to skip
recursively calling find_overload_match).
> + }
> +}
> +
> +/* Checks if the N1-th method in FNS_PTR1 has exactly the same parameters
> + as that of the N2-th method in FNS_PTR2.
> + Returns 1 is equal, zero otherwise. */
> +
> +static int
> +equal_param_types_p (struct fn_field *fns_ptr1, int n1,
> + struct fn_field *fns_ptr2, int n2)
> +{
> + int i;
> +
> + if (TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr1, n1))
> + != TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr2, n2)))
> + return 0;
> +
> + for (i = 1; i < TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr1, n1)); i++)
> + {
> + struct type *type1 = TYPE_FN_FIELD_ARGS (fns_ptr1, n1)[i].type;
> + struct type *type2 = TYPE_FN_FIELD_ARGS (fns_ptr2, n2)[i].type;
> +
> + if (!types_deeply_equal (type1, type2))
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +/* Checks if the derived type DTYPE hides a method NAME of its base class.
> + The base class method is the M-th method in FNS_PTR. */
> +
> +static int
> +derived_hides_base_method (struct type *dtype, const char *name,
derived_hides_base_method_p
> + struct fn_field *fns_ptr, int m)
> +{
> + int i, ret_val = 0;
> +
> + dtype = check_typedef (dtype);
> + gdb_assert (TYPE_CODE (dtype) == TYPE_CODE_STRUCT);
> +
> + for (i = 0; i < TYPE_NFN_FIELDS (dtype); i++)
> + {
> + const char *fn_field_name = TYPE_FN_FIELDLIST_NAME (dtype, i);
> +
> + if (fn_field_name && (strcmp_iw (fn_field_name, name) == 0))
> + {
> + int j;
> +
> + /* If a method with the same name is found in the dynamic type,
> + then assume that it hides the base method until a method with
> + matching param types is found. */
> + ret_val = 1;
> +
> + for (j = 0; j < TYPE_FN_FIELDLIST_LENGTH (dtype, i); j++)
> + {
> + if (equal_param_types_p (TYPE_FN_FIELDLIST1 (dtype, i), j,
> + fns_ptr, m))
> + return 0;
> + }
> + }
> + }
> +
> + return ret_val;
> }
>
> /* Given an array of arguments (ARGS) (which includes an
> @@ -2418,16 +2573,24 @@ find_overload_match (struct value **args, int nargs,
> /* Index of best overloaded function. */
> int func_oload_champ = -1;
> int method_oload_champ = -1;
> + int src_method_oload_champ = -1;
> + int src_method_oload_champ_orig = -1;
> + int ext_method_oload_champ = -1;
> + int src_and_ext_equal = 0;
>
> /* The measure for the current best match. */
> struct badness_vector *method_badness = NULL;
> struct badness_vector *func_badness = NULL;
> + struct badness_vector *ext_method_badness = NULL;
> + struct badness_vector *src_method_badness = NULL;
>
> struct value *temp = obj;
> /* For methods, the list of overloaded methods. */
> struct fn_field *fns_ptr = NULL;
> /* For non-methods, the list of overloaded function symbols. */
> struct symbol **oload_syms = NULL;
> + /* For xmethods, the VEC of xmethod workers. */
> + VEC (xmethod_worker_ptr) *xm_worker_vec = NULL;
> /* Number of overloaded instances being considered. */
> int num_fns = 0;
> struct type *basetype = NULL;
> @@ -2439,6 +2602,8 @@ find_overload_match (struct value **args, int nargs,
> const char *func_name = NULL;
> enum oload_classification match_quality;
> enum oload_classification method_match_quality = INCOMPATIBLE;
> + enum oload_classification src_method_match_quality = INCOMPATIBLE;
> + enum oload_classification ext_method_match_quality = INCOMPATIBLE;
> enum oload_classification func_match_quality = INCOMPATIBLE;
>
> /* Get the list of overloaded methods or functions. */
> @@ -2467,12 +2632,11 @@ find_overload_match (struct value **args, int nargs,
> }
>
> /* Retrieve the list of methods with the name NAME. */
> - fns_ptr = value_find_oload_method_list (&temp, name,
> - 0, &num_fns,
> - &basetype, &boffset);
> + value_find_oload_method_list (&temp, name, 0, &fns_ptr, &num_fns,
> + &xm_worker_vec, &basetype, &boffset);
> /* If this is a method only search, and no methods were found
> the search has faild. */
> - if (method == METHOD && (!fns_ptr || !num_fns))
> + if (method == METHOD && (!fns_ptr || !num_fns) && !xm_worker_vec)
> error (_("Couldn't find method %s%s%s"),
> obj_type_name,
> (obj_type_name && *obj_type_name) ? "::" : "",
> @@ -2483,18 +2647,87 @@ find_overload_match (struct value **args, int nargs,
> if (fns_ptr)
> {
> gdb_assert (TYPE_DOMAIN_TYPE (fns_ptr[0].type) != NULL);
> - method_oload_champ = find_oload_champ (args, nargs,
> - num_fns, fns_ptr,
> - NULL, &method_badness);
>
> - method_match_quality =
> - classify_oload_match (method_badness, nargs,
> - oload_method_static_p (fns_ptr,
> - method_oload_champ));
> + src_method_oload_champ = find_oload_champ (args, nargs,
> + num_fns, fns_ptr, NULL,
> + NULL, &src_method_badness);
> +
> + src_method_match_quality = classify_oload_match
> + (src_method_badness, nargs,
> + oload_method_static_p (fns_ptr, src_method_oload_champ));
>
> - make_cleanup (xfree, method_badness);
> + make_cleanup (xfree, src_method_badness);
> }
>
> + if (VEC_length (xmethod_worker_ptr, xm_worker_vec) > 0)
> + {
> + ext_method_oload_champ = find_oload_champ (args, nargs,
> + 0, NULL, xm_worker_vec,
> + NULL, &ext_method_badness);
> + ext_method_match_quality = classify_oload_match (ext_method_badness,
> + nargs, 0);
> + make_cleanup (xfree, ext_method_badness);
> + make_cleanup (free_xmethod_worker_vec, xm_worker_vec);
> + }
> +
> + if (src_method_oload_champ >= 0 && ext_method_oload_champ >= 0)
> + {
> + switch (compare_badness (ext_method_badness, src_method_badness))
> + {
> + case 0: /* Src method and xmethod are equally good. */
> + src_and_ext_equal = 1;
> + /* If src method and xmethod are equally good, then
> + xmethod should be the winner. Hence, fall through to the
> + case where a xmethod is better than the source
> + method, except when the xmethod match quality is
> + non-standard. */
> + /* FALLTHROUGH */
> + case 1: /* Src method and ext method are incompatible. */
> + /* If ext method match is not standard, then let source method
> + win. Otherwise, fallthrough to let xmethod win. */
> + if (ext_method_match_quality != STANDARD)
> + {
> + method_oload_champ = src_method_oload_champ;
> + method_badness = src_method_badness;
> + ext_method_oload_champ = -1;
> + method_match_quality = src_method_match_quality;
> + break;
> + }
> + /* FALLTHROUGH */
> + case 2: /* Ext method is champion. */
> + method_oload_champ = ext_method_oload_champ;
> + method_badness = ext_method_badness;
> + /* We save the source overload champ index so that it can be
> + used to determine whether the source method is virtual
> + later in this function. */
> + src_method_oload_champ_orig = src_method_oload_champ;
> + src_method_oload_champ = -1;
> + method_match_quality = ext_method_match_quality;
> + break;
> + case 3: /* Src method is champion. */
> + method_oload_champ = src_method_oload_champ;
> + method_badness = src_method_badness;
> + ext_method_oload_champ = -1;
> + method_match_quality = src_method_match_quality;
> + break;
> + default:
> + gdb_assert_not_reached ("Internal error: unexpected overload "
No need for "Internal error" in the text here.
> + "comparison result");
> + break;
> + }
> + }
> + else if (src_method_oload_champ >= 0)
> + {
> + method_oload_champ = src_method_oload_champ;
> + method_badness = src_method_badness;
> + method_match_quality = src_method_match_quality;
> + }
> + else if (ext_method_oload_champ >= 0)
> + {
> + method_oload_champ = ext_method_oload_champ;
> + method_badness = ext_method_badness;
> + method_match_quality = ext_method_match_quality;
> + }
> }
>
> if (method == NON_METHOD || method == BOTH)
> @@ -2637,21 +2870,6 @@ find_overload_match (struct value **args, int nargs,
> func_name);
> }
>
> - if (staticp != NULL)
> - *staticp = oload_method_static_p (fns_ptr, method_oload_champ);
> -
> - if (method_oload_champ >= 0)
> - {
> - if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ))
> - *valp = value_virtual_fn_field (&temp, fns_ptr, method_oload_champ,
> - basetype, boffset);
> - else
> - *valp = value_fn_field (&temp, fns_ptr, method_oload_champ,
> - basetype, boffset);
> - }
> - else
> - *symp = oload_syms[func_oload_champ];
> -
> if (objp)
> {
> struct type *temp_type = check_typedef (value_type (temp));
> @@ -2661,10 +2879,112 @@ find_overload_match (struct value **args, int nargs,
> && (TYPE_CODE (objtype) == TYPE_CODE_PTR
> || TYPE_CODE (objtype) == TYPE_CODE_REF))
> {
> - temp = value_addr (temp);
> + *objp = value_addr (temp);
> + }
> + else
> + *objp = temp;
> + }
I don't understand this part (from "@@ -2661,10 +2879,112 @@" to here).
What's it for? AFAICT it's just a gratuitous change.
> +
> + if (staticp != NULL)
> + *staticp = oload_method_static_p (fns_ptr, method_oload_champ);
> +
> + if (method_oload_champ >= 0)
> + {
> + if (src_method_oload_champ >= 0)
> + {
> + /* Even if the source method was the winner, it could be a virtual
> + function. In such a case, we should look for the possibility of
> + xmethods defined on the object's dynamic type. */
I'm finding it hard to reason about this code.
It's possibly correct, but I can't say if there are any gotchas.
IWBN if we could look at this differently that would lead to a
massive simplification (maybe wishful thinking, maybe not).
I'm thinking we should leave handling xmethod lookup by dynamic type
to later, get this patch in without it, and then add that functionality
in a separate pass.
That would mean removing
value_has_indirect_dynamic_type, derived_hides_base_method,
cast_args_to_param_types, and the recursive calls to find_overload_match.
At least for now. Once patches 1,2,3,4 are checked in,
we can then revisit this part.
> + if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ))
> + {
> + struct type *dtype;
> +
> + dtype = value_has_indirect_dynamic_type (args[0]);
> + /* Look for better methods in the dynamic type only if the
> + dynamic type does not hide the base class virtual method. */
> + if (dtype != NULL
> + && !derived_hides_base_method (TYPE_TARGET_TYPE (dtype),
> + name, fns_ptr,
> + method_oload_champ))
> + {
> + /* If the object has a dynamic type, then look for matching
> + methods for its dynamic type. */
> + args[0] = value_cast (dtype, args[0]);
> + do_cleanups (all_cleanups);
> + /* Even if the derived class does not hide the base class
> + method, it could define another method with the same name
> + but with compatible parameters. Avoid the chance of
> + picking up the wrong function by explicitly casting the
> + args to the virtual function param types.
> +
> + Example:
> +
> + class base
> + {
> + public:
> + virtual int foo (char i);
> + };
> +
> + class derived : public base
> + {
> + public:
> + virtual int foo (char i);
> + int foo (int i);
> + };
> +
> + If the arg to base::foo was as int, then looking for
> + matching methods in the dynamic object will match foo(int)
> + and not foo (char) even though the derived class overrides
> + (but not hide) the base class virtual method. */
> + cast_args_to_param_types (args, nargs, fns_ptr,
> + method_oload_champ);
> + return find_overload_match (args, nargs, name, method,
> + objp, fsym, valp, symp,
> + staticp, no_adl);
> + }
> + else
> + *valp = value_virtual_fn_field (&temp, fns_ptr,
> + method_oload_champ,
> + basetype, boffset);
> + }
> + else
> + *valp = value_fn_field (&temp, fns_ptr, method_oload_champ,
> + basetype, boffset);
> + }
> + else
> + {
> + /* Xmethods cannot be virtual. However, if an xmethod is as
> + good as the source method, and if the source method is virtual, we
> + should look for the possibility of better matching methods defined
> + for the dynamic type of the object. */
> + if (src_and_ext_equal
> + && TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, src_method_oload_champ_orig))
> + {
> + struct type *dtype;
> +
> + dtype = value_has_indirect_dynamic_type (args[0]);
> + if (dtype != NULL
> + && !derived_hides_base_method (TYPE_TARGET_TYPE (dtype),
> + name, fns_ptr,
> + src_method_oload_champ_orig))
> + {
> + args[0] = value_cast (dtype, args[0]);
> + do_cleanups (all_cleanups);
> + cast_args_to_param_types (args, nargs, fns_ptr,
> + src_method_oload_champ_orig);
> + return find_overload_match (args, nargs, name, method,
> + objp, fsym, valp, symp,
> + staticp, no_adl);
> + }
> + }
> +
> + *valp = value_of_xmethod (clone_xmethod_worker
> + (VEC_index (xmethod_worker_ptr, xm_worker_vec,
> + ext_method_oload_champ)));
> }
> - *objp = temp;
> }
> + else
> + *symp = oload_syms[func_oload_champ];
>
> do_cleanups (all_cleanups);
>
> @@ -2799,7 +3119,7 @@ find_oload_champ_namespace_loop (struct value **args, int nargs,
> ++num_fns;
>
> new_oload_champ = find_oload_champ (args, nargs, num_fns,
> - NULL, new_oload_syms,
> + NULL, NULL, new_oload_syms,
> &new_oload_champ_bv);
>
> /* Case 1: We found a good match. Free earlier matches (if any),
> @@ -2837,9 +3157,13 @@ find_oload_champ_namespace_loop (struct value **args, int nargs,
>
> /* Look for a function to take NARGS args of ARGS. Find
> the best match from among the overloaded methods or functions
> - given by FNS_PTR or OLOAD_SYMS, respectively. One, and only one of
> - FNS_PTR and OLOAD_SYMS can be non-NULL. The number of
> - methods/functions in the non-NULL list is given by NUM_FNS.
> + given by FNS_PTR or OLOAD_SYMS or DM_WORKER_VEC, respectively.
> + One, and only one of FNS_PTR, OLOAD_SYMS and DM_WORKER_VEC can be
> + non-NULL.
> +
> + If XM_WORKER_VEC is NULL, then the length of the arrays FNS_PTR
> + or OLOAD_SYMS (whichever is non-NULL) is specified in NUM_FNS.
> +
> Return the index of the best match; store an indication of the
> quality of the match in OLOAD_CHAMP_BV.
>
> @@ -2848,10 +3172,13 @@ find_oload_champ_namespace_loop (struct value **args, int nargs,
> static int
> find_oload_champ (struct value **args, int nargs,
> int num_fns, struct fn_field *fns_ptr,
> + VEC (xmethod_worker_ptr) *xm_worker_vec,
> struct symbol **oload_syms,
> struct badness_vector **oload_champ_bv)
> {
> int ix;
> + int fn_count;
> + int xm_worker_vec_n = VEC_length (xmethod_worker_ptr, xm_worker_vec);
> /* A measure of how good an overloaded instance is. */
> struct badness_vector *bv;
> /* Index of best overloaded function. */
> @@ -2861,40 +3188,49 @@ find_oload_champ (struct value **args, int nargs,
> /* 0 => no ambiguity, 1 => two good funcs, 2 => incomparable funcs. */
>
> /* A champion can be found among methods alone, or among functions
> - alone, but not both. */
> - gdb_assert ((fns_ptr != NULL) + (oload_syms != NULL) == 1);
> + alone, or in xmethods alone, but not in more than one of these
> + groups. */
> + gdb_assert ((fns_ptr != NULL) + (oload_syms != NULL) + (xm_worker_vec != NULL)
> + == 1);
>
> *oload_champ_bv = NULL;
>
> + fn_count = (xm_worker_vec != NULL
> + ? VEC_length (xmethod_worker_ptr, xm_worker_vec)
> + : num_fns);
> /* Consider each candidate in turn. */
> - for (ix = 0; ix < num_fns; ix++)
> + for (ix = 0; ix < fn_count; ix++)
> {
> int jj;
> - int static_offset;
> + int static_offset = 0;
> int nparms;
> struct type **parm_types;
> + struct xmethod_worker *worker = NULL;
>
> - if (fns_ptr != NULL)
> + if (xm_worker_vec != NULL)
> {
> - nparms = TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr, ix));
> - static_offset = oload_method_static_p (fns_ptr, ix);
> + worker = VEC_index (xmethod_worker_ptr, xm_worker_vec, ix);
> + parm_types = get_xmethod_arg_types (worker, &nparms);
> }
> else
> {
> - /* If it's not a method, this is the proper place. */
> - nparms = TYPE_NFIELDS (SYMBOL_TYPE (oload_syms[ix]));
> - static_offset = 0;
> + if (fns_ptr != NULL)
> + {
> + nparms = TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr, ix));
> + static_offset = oload_method_static_p (fns_ptr, ix);
> + }
> + else
> + nparms = TYPE_NFIELDS (SYMBOL_TYPE (oload_syms[ix]));
> +
> + parm_types = (struct type **)
> + xmalloc (nparms * (sizeof (struct type *)));
> + for (jj = 0; jj < nparms; jj++)
> + parm_types[jj] = (fns_ptr != NULL
> + ? (TYPE_FN_FIELD_ARGS (fns_ptr, ix)[jj].type)
> + : TYPE_FIELD_TYPE (SYMBOL_TYPE (oload_syms[ix]),
> + jj));
> }
>
> - /* Prepare array of parameter types. */
> - parm_types = (struct type **)
> - xmalloc (nparms * (sizeof (struct type *)));
> - for (jj = 0; jj < nparms; jj++)
> - parm_types[jj] = (fns_ptr != NULL
> - ? (TYPE_FN_FIELD_ARGS (fns_ptr, ix)[jj].type)
> - : TYPE_FIELD_TYPE (SYMBOL_TYPE (oload_syms[ix]),
> - jj));
> -
> /* Compare parameter types to supplied argument types. Skip
> THIS for static methods. */
> bv = rank_function (parm_types, nparms,
> @@ -2928,10 +3264,14 @@ find_oload_champ (struct value **args, int nargs,
> xfree (parm_types);
> if (overload_debug)
> {
> - if (fns_ptr)
> + if (fns_ptr != NULL)
> fprintf_filtered (gdb_stderr,
> "Overloaded method instance %s, # of parms %d\n",
> fns_ptr[ix].physname, nparms);
> + else if (xm_worker_vec != NULL)
> + fprintf_filtered (gdb_stderr,
> + "Xmethod worker, # of parms %d\n",
> + nparms);
> else
> fprintf_filtered (gdb_stderr,
> "Overloaded function instance "
--
/dje