From patchwork Wed Nov 21 12:48:07 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 30241 Received: (qmail 95739 invoked by alias); 21 Nov 2018 12:48:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 94587 invoked by uid 89); 21 Nov 2018 12:48:29 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 21 Nov 2018 12:48:26 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 339533082E62; Wed, 21 Nov 2018 12:48:09 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4C0A317D23; Wed, 21 Nov 2018 12:48:08 +0000 (UTC) Subject: Re: [PATCH 5/6] valops.c: Some more gdb::array_view To: Simon Marchi , "gdb-patches@sourceware.org" References: <20181015151115.6356-1-palves@redhat.com> <20181015151115.6356-6-palves@redhat.com> <4fcedad5-fb6a-11ed-d24f-9e9a3f6339e7@ericsson.com> From: Pedro Alves Message-ID: <8f47e046-17b0-6070-0743-1d7e87535f9f@redhat.com> Date: Wed, 21 Nov 2018 12:48:07 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <4fcedad5-fb6a-11ed-d24f-9e9a3f6339e7@ericsson.com> On 10/17/2018 07:03 PM, Simon Marchi wrote: > On 2018-10-15 11:11 a.m., Pedro Alves wrote: >> This commit replaces some more use of pointer+length pairs in the >> overload resolution code with gdb::array_view. >> >> find_oload_champ's interface is simplified/normalized: the xmethods >> parameter is converted from std::vector to array pointer, and then the >> num_fns parameter is always passed in, no matter the array which is >> non-NULL. I tweaked the formatting of callers a little bit here and >> there so that the 3 optional parameters are all in the same line. (I >> tried making the 3 optional array parameters be array_views, but the >> resulting code didn't look as nice.) > > I don't really understand what's happening in that code, but it looks to me > like the behavior is kept, which is good. You can look at find_oload_champ as a template function -- it finds the best overload match from a given set of functions. Now, I'm calling it a "template", because there are 3 different sources of functions to consider: free functions, class methods, or xmethods, and each of those are arrays/vectors of different types. Only one of those sources can be passed down to the function. The non-normalized thing with the function currently is that two of the sources are passed as "pointer to array", while one source (xmethods) is passed as an std::vector. When you pass down one of the "pointer to array" sources (functions or class methods), you need to pass the array length as well in NUM_FDS. But when you pass down xmethods, you don't pass down the vector's length, because the function gets it from the vector itself. The patch essentially makes the xmethod source case be passed by "pointer to array" as well, so that all sources are implemented the same way. And in turns means that we now pass down the array length via NUM_FDS for xmethods too. > >> @@ -2572,23 +2564,28 @@ find_overload_match (gdb::array_view args, >> /* If we are dealing with stub method types, they should have >> been resolved by find_method_list via >> value_find_oload_method_list above. */ >> - if (fns_ptr) >> + if (!fns_list.empty ()) >> { >> - gdb_assert (TYPE_SELF_TYPE (fns_ptr[0].type) != NULL); >> + gdb_assert (TYPE_SELF_TYPE (fns_list[0].type) != NULL); >> >> - src_method_oload_champ = find_oload_champ (args, >> - num_fns, fns_ptr, NULL, >> - NULL, &src_method_badness); >> + src_method_oload_champ >> + = find_oload_champ (args, >> + fns_list.size (), >> + fns_list.data (), NULL, NULL, >> + &src_method_badness); >> >> src_method_match_quality = classify_oload_match >> (src_method_badness, args.size (), >> - oload_method_static_p (fns_ptr, src_method_oload_champ)); >> + oload_method_static_p (fns_list.data (), src_method_oload_champ)); >> } >> >> if (!xm_worker_vec.empty ()) >> { >> - ext_method_oload_champ = find_oload_champ (args, 0, NULL, &xm_worker_vec, >> - NULL, &ext_method_badness); >> + ext_method_oload_champ >> + = find_oload_champ (args, >> + xm_worker_vec.size (), >> + NULL, xm_worker_vec.data (), NULL, >> + &ext_method_badness); > > The previous code passed a size of 0... was it an error? Nope. See above. > >> @@ -2984,20 +2982,20 @@ find_oload_champ_namespace_loop (gdb::array_view args, >> or XM_WORKER_VEC, respectively. One, and only one of FNS_PTR, >> OLOAD_SYMS and XM_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. >> + NUM_FNS is the length of the array pointed at by FNS_PTR, >> + OLOAD_SYMS of XM_WORKER_VEC, whichever is non-NULL. > > of -> or Fixed, thanks. Here's what I merged. I merged patch 6/6 as well. From 85cca2bcbc7833b33d4b61d7b7e0e75b9afa063b Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 21 Nov 2018 11:55:14 +0000 Subject: [PATCH 5/6] valops.c: Some more gdb::array_view This commit replaces some more use of pointer+length pairs in the overload resolution code with gdb::array_view. find_oload_champ's interface is simplified/normalized: the xmethods parameter is converted from std::vector to array pointer, and then the num_fns parameter is always passed in, no matter the array which is non-NULL. I tweaked the formatting of callers a little bit here and there so that the 3 optional parameters are all in the same line. (I tried making the 3 optional array parameters be array_views, but the resulting code didn't look as nice.) gdb/ChangeLog: 2018-11-21 Pedro Alves * valops.c (find_method_list): Replace pointer and length parameters with an gdb::array_view. Adjust. (value_find_oload_method_list): Likewise. (find_overload_match): Use gdb::array_view for methods list. Adjust to find_oload_champ interface change. (find_oload_champ): 'xm_worker_vec' parameter now a pointer/array. 'num_fns' parameter now a size_t. Eliminate 'fn_count' local. --- gdb/ChangeLog | 10 ++++++ gdb/valops.c | 101 +++++++++++++++++++++++++++------------------------------- 2 files changed, 57 insertions(+), 54 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 5ae9ae7b1b..ff9eed7dee 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2018-11-21 Pedro Alves + + * valops.c (find_method_list): Replace pointer and length + parameters with an gdb::array_view. Adjust. + (value_find_oload_method_list): Likewise. + (find_overload_match): Use gdb::array_view for methods list. + Adjust to find_oload_champ interface change. + (find_oload_champ): 'xm_worker_vec' parameter now a pointer/array. + 'num_fns' parameter now a size_t. Eliminate 'fn_count' local. + 2018-11-21 Pedro Alves * gdbtypes.c (compare_badness): Change type of parameters to const diff --git a/gdb/valops.c b/gdb/valops.c index 1b4a8e123c..1d68c04bfe 100644 --- a/gdb/valops.c +++ b/gdb/valops.c @@ -66,10 +66,12 @@ static int find_oload_champ_namespace_loop (gdb::array_view args, badness_vector *, int *, const int no_adl); -static int find_oload_champ (gdb::array_view args, int, - struct fn_field *, - const std::vector *, - struct symbol **, badness_vector *); +static int find_oload_champ (gdb::array_view args, + size_t num_fns, + fn_field *fns_ptr, + xmethod_worker_up *xm_worker_vec, + symbol **oload_syms, + badness_vector *oload_champ_bv); static int oload_method_static_p (struct fn_field *, int); @@ -95,11 +97,6 @@ static CORE_ADDR allocate_space_in_inferior (int); static struct value *cast_into_complex (struct type *, struct value *); -static void find_method_list (struct value **, const char *, - LONGEST, struct type *, struct fn_field **, int *, - std::vector *, - struct type **, LONGEST *); - int overload_resolution = 0; static void show_overload_resolution (struct ui_file *file, int from_tty, @@ -2292,7 +2289,7 @@ value_union_variant (struct type *union_type, const gdb_byte *contents) } /* Search through the methods of an object (and its bases) to find a - specified method. Return the pointer to the fn_field list FN_LIST of + specified method. Return a reference 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 @@ -2316,7 +2313,7 @@ value_union_variant (struct type *union_type, const gdb_byte *contents) static void find_method_list (struct value **argp, const char *method, LONGEST offset, struct type *type, - struct fn_field **fn_list, int *num_fns, + gdb::array_view *fn_list, std::vector *xm_worker_vec, struct type **basetype, LONGEST *boffset) { @@ -2330,7 +2327,7 @@ find_method_list (struct value **argp, const char *method, 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) + if (fn_list->empty ()) { for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; i--) { @@ -2341,9 +2338,8 @@ find_method_list (struct value **argp, const char *method, { int len = TYPE_FN_FIELDLIST_LENGTH (type, i); f = TYPE_FN_FIELDLIST1 (type, i); - *fn_list = f; + *fn_list = gdb::make_array_view (f, len); - *num_fns = len; *basetype = type; *boffset = offset; @@ -2385,7 +2381,7 @@ find_method_list (struct value **argp, const char *method, } find_method_list (argp, method, base_offset + offset, - TYPE_BASECLASS (type, i), fn_list, num_fns, + TYPE_BASECLASS (type, i), fn_list, xm_worker_vec, basetype, boffset); } } @@ -2398,9 +2394,8 @@ find_method_list (struct value **argp, const char *method, 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 is the pointer to matching overloaded instances defined in + FN_LIST is the list of matching overloaded instances defined in source language. - NUM_FNS is the number of overloaded instances. XM_WORKER_VEC is the vector of matching xmethod workers defined in extension languages. BASETYPE is set to the type of the base subobject that defines the @@ -2409,8 +2404,8 @@ find_method_list (struct value **argp, const char *method, static void value_find_oload_method_list (struct value **argp, const char *method, - LONGEST offset, struct fn_field **fn_list, - int *num_fns, + LONGEST offset, + gdb::array_view *fn_list, std::vector *xm_worker_vec, struct type **basetype, LONGEST *boffset) { @@ -2436,11 +2431,10 @@ value_find_oload_method_list (struct value **argp, const char *method, gdb_assert (fn_list != NULL && xm_worker_vec != NULL); /* Clear the lists. */ - *fn_list = NULL; - *num_fns = 0; + *fn_list = {}; xm_worker_vec->clear (); - find_method_list (argp, method, 0, t, fn_list, num_fns, xm_worker_vec, + find_method_list (argp, method, 0, t, fn_list, xm_worker_vec, basetype, boffset); } @@ -2514,13 +2508,11 @@ find_overload_match (gdb::array_view args, struct value *temp = obj; /* For methods, the list of overloaded methods. */ - struct fn_field *fns_ptr = NULL; + gdb::array_view fns_list; /* For non-methods, the list of overloaded function symbols. */ std::vector oload_syms; /* For xmethods, the vector of xmethod workers. */ std::vector xm_worker_vec; - /* Number of overloaded instances being considered. */ - int num_fns = 0; struct type *basetype = NULL; LONGEST boffset; @@ -2560,11 +2552,11 @@ find_overload_match (gdb::array_view args, } /* Retrieve the list of methods with the name NAME. */ - value_find_oload_method_list (&temp, name, 0, &fns_ptr, &num_fns, + value_find_oload_method_list (&temp, name, 0, &fns_list, &xm_worker_vec, &basetype, &boffset); /* If this is a method only search, and no methods were found the search has failed. */ - if (method == METHOD && (!fns_ptr || !num_fns) && xm_worker_vec.empty ()) + if (method == METHOD && fns_list.empty () && xm_worker_vec.empty ()) error (_("Couldn't find method %s%s%s"), obj_type_name, (obj_type_name && *obj_type_name) ? "::" : "", @@ -2572,23 +2564,28 @@ find_overload_match (gdb::array_view args, /* If we are dealing with stub method types, they should have been resolved by find_method_list via value_find_oload_method_list above. */ - if (fns_ptr) + if (!fns_list.empty ()) { - gdb_assert (TYPE_SELF_TYPE (fns_ptr[0].type) != NULL); + gdb_assert (TYPE_SELF_TYPE (fns_list[0].type) != NULL); - src_method_oload_champ = find_oload_champ (args, - num_fns, fns_ptr, NULL, - NULL, &src_method_badness); + src_method_oload_champ + = find_oload_champ (args, + fns_list.size (), + fns_list.data (), NULL, NULL, + &src_method_badness); src_method_match_quality = classify_oload_match (src_method_badness, args.size (), - oload_method_static_p (fns_ptr, src_method_oload_champ)); + oload_method_static_p (fns_list.data (), src_method_oload_champ)); } if (!xm_worker_vec.empty ()) { - ext_method_oload_champ = find_oload_champ (args, 0, NULL, &xm_worker_vec, - NULL, &ext_method_badness); + ext_method_oload_champ + = find_oload_champ (args, + xm_worker_vec.size (), + NULL, xm_worker_vec.data (), NULL, + &ext_method_badness); ext_method_match_quality = classify_oload_match (ext_method_badness, args.size (), 0); } @@ -2787,22 +2784,22 @@ find_overload_match (gdb::array_view args, } if (staticp != NULL) - *staticp = oload_method_static_p (fns_ptr, method_oload_champ); + *staticp = oload_method_static_p (fns_list.data (), method_oload_champ); if (method_oload_champ >= 0) { if (src_method_oload_champ >= 0) { - if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ) + if (TYPE_FN_FIELD_VIRTUAL_P (fns_list, method_oload_champ) && noside != EVAL_AVOID_SIDE_EFFECTS) { - *valp = value_virtual_fn_field (&temp, fns_ptr, + *valp = value_virtual_fn_field (&temp, fns_list.data (), method_oload_champ, basetype, boffset); } else - *valp = value_fn_field (&temp, fns_ptr, method_oload_champ, - basetype, boffset); + *valp = value_fn_field (&temp, fns_list.data (), + method_oload_champ, basetype, boffset); } else *valp = value_from_xmethod @@ -2942,7 +2939,8 @@ find_oload_champ_namespace_loop (gdb::array_view args, } badness_vector new_oload_champ_bv; - new_oload_champ = find_oload_champ (args, new_oload_syms.size (), + new_oload_champ = find_oload_champ (args, + new_oload_syms.size (), NULL, NULL, new_oload_syms.data (), &new_oload_champ_bv); @@ -2979,20 +2977,20 @@ find_oload_champ_namespace_loop (gdb::array_view args, or XM_WORKER_VEC, respectively. One, and only one of FNS_PTR, OLOAD_SYMS and XM_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. + NUM_FNS is the length of the array pointed at by FNS_PTR, + OLOAD_SYMS or XM_WORKER_VEC, whichever is non-NULL. Return the index of the best match; store an indication of the quality of the match in OLOAD_CHAMP_BV. */ static int find_oload_champ (gdb::array_view args, - int num_fns, struct fn_field *fns_ptr, - const std::vector *xm_worker_vec, - struct symbol **oload_syms, + size_t num_fns, + fn_field *fns_ptr, + xmethod_worker_up *xm_worker_vec, + symbol **oload_syms, badness_vector *oload_champ_bv) { - int ix; /* A measure of how good an overloaded instance is. */ badness_vector bv; /* Index of best overloaded function. */ @@ -3007,20 +3005,15 @@ find_oload_champ (gdb::array_view args, gdb_assert ((fns_ptr != NULL) + (oload_syms != NULL) + (xm_worker_vec != NULL) == 1); - int fn_count = xm_worker_vec != NULL ? xm_worker_vec->size () : num_fns; - /* Consider each candidate in turn. */ - for (ix = 0; ix < fn_count; ix++) + for (size_t ix = 0; ix < num_fns; ix++) { int jj; int static_offset = 0; std::vector parm_types; if (xm_worker_vec != NULL) - { - xmethod_worker *worker = (*xm_worker_vec)[ix].get (); - parm_types = worker->get_arg_types (); - } + parm_types = xm_worker_vec[ix]->get_arg_types (); else { size_t nparms;