From patchwork Wed Nov 21 12:47:10 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 30239 Received: (qmail 61107 invoked by alias); 21 Nov 2018 12:47:16 -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 60419 invoked by uid 89); 21 Nov 2018 12:47:16 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=workers, 14127, quit, cust 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:47:13 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E1D91308123E; Wed, 21 Nov 2018 12:47:11 +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 00A2560478; Wed, 21 Nov 2018 12:47:10 +0000 (UTC) Subject: Re: [PATCH 3/6] Eliminate make_symbol_overload_list-related globals & cleanup To: Simon Marchi , "gdb-patches@sourceware.org" References: <20181015151115.6356-1-palves@redhat.com> <20181015151115.6356-4-palves@redhat.com> <818ad030-6665-71c7-006d-77ad6b5895c8@ericsson.com> From: Pedro Alves Message-ID: Date: Wed, 21 Nov 2018 12:47:10 +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: <818ad030-6665-71c7-006d-77ad6b5895c8@ericsson.com> On 10/17/2018 06:25 PM, Simon Marchi wrote: > On 2018-10-15 11:11 a.m., Pedro Alves wrote: >> This gets rid of a few globals and a cleanup. >> >> make_symbol_overload_list & friends currently maintain a global >> open-coded vector. Reimplement that with a std::vector, trickled down >> through the functions. Rename a few functions from "make_" to "add_" >> for clarity. > > LGTM, my only comment would be to avoid non-const passing by reference, as in > the previous patch. Done. Here's what I merged. From 0891c3cc132495ad7b323896efae4f91eca87c6c Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 21 Nov 2018 11:55:13 +0000 Subject: [PATCH 3/6] Eliminate make_symbol_overload_list-related globals & cleanup This gets rid of a few globals and a cleanup. make_symbol_overload_list & friends currently maintain a global open-coded vector. Reimplement that with a std::vector, trickled down through the functions. Rename a few functions from "make_" to "add_" for clarity. gdb/ChangeLog: 2018-11-21 Pedro Alves * cp-support.c (sym_return_val_size, sym_return_val_index) (sym_return_val): Delete. (overload_list_add_symbol): Add std::vector parameter. Adjust to add to the vector. (make_symbol_overload_list): Adjust to return a std::vector instead of maintaining a global open coded vector. (make_symbol_overload_list_block): Add std::vector parameter. (make_symbol_overload_list_block): Rename to ... (add_symbol_overload_list_block): ... this and add std::vector parameter. (make_symbol_overload_list_namespace): Rename to ... (add_symbol_overload_list_namespace): ... this and add std::vector parameter. (make_symbol_overload_list_adl_namespace): Rename to ... (add_symbol_overload_list_adl_namespace): ... this and add std::vector parameter. (make_symbol_overload_list_adl): Delete. (add_symbol_overload_list_adl): New. (make_symbol_overload_list_using): Rename to ... (add_symbol_overload_list_using): ... this and add std::vector parameter. (make_symbol_overload_list_qualified): Rename to ... (add_symbol_overload_list_qualified): ... this and add std::vector parameter. * cp-support.h: Include "common/array-view.h" and . (make_symbol_overload_list): Change return type to std::vector. (make_symbol_overload_list_adl): Delete declaration. (add_symbol_overload_list_adl): New declaration. * valops.c (find_overload_match): Local 'oload_syms' now a std::vector. (find_oload_champ_namespace): 'oload_syms' parameter now a std::vector pointer. (find_oload_champ_namespace_loop): 'oload_syms' parameter now a std::vector pointer. Adjust to new make_symbol_overload_list interface. --- gdb/ChangeLog | 38 +++++++++++++++ gdb/cp-support.c | 142 ++++++++++++++++++++++++------------------------------- gdb/cp-support.h | 13 +++-- gdb/valops.c | 37 ++++++--------- 4 files changed, 124 insertions(+), 106 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 27da6431b3..e808dd5dab 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,41 @@ +2018-11-21 Pedro Alves + + * cp-support.c (sym_return_val_size, sym_return_val_index) + (sym_return_val): Delete. + (overload_list_add_symbol): Add std::vector parameter. Adjust to + add to the vector. + (make_symbol_overload_list): Adjust to return a std::vector + instead of maintaining a global open coded vector. + (make_symbol_overload_list_block): Add std::vector parameter. + (make_symbol_overload_list_block): Rename to ... + (add_symbol_overload_list_block): ... this and add std::vector + parameter. + (make_symbol_overload_list_namespace): Rename to ... + (add_symbol_overload_list_namespace): ... this and add std::vector + parameter. + (make_symbol_overload_list_adl_namespace): Rename to ... + (add_symbol_overload_list_adl_namespace): ... this and add + std::vector parameter. + (make_symbol_overload_list_adl): Delete. + (add_symbol_overload_list_adl): New. + (make_symbol_overload_list_using): Rename to ... + (add_symbol_overload_list_using): ... this and add std::vector + parameter. + (make_symbol_overload_list_qualified): Rename to ... + (add_symbol_overload_list_qualified): ... this and add std::vector + parameter. + * cp-support.h: Include "common/array-view.h" and . + (make_symbol_overload_list): Change return type to std::vector. + (make_symbol_overload_list_adl): Delete declaration. + (add_symbol_overload_list_adl): New declaration. + * valops.c (find_overload_match): Local 'oload_syms' now a + std::vector. + (find_oload_champ_namespace): 'oload_syms' parameter now a + std::vector pointer. + (find_oload_champ_namespace_loop): 'oload_syms' parameter now a + std::vector pointer. Adjust to new make_symbol_overload_list + interface. + 2018-11-21 Pedro Alves * common/array-view.h (array_view::splice(size_type, size_t)): New. diff --git a/gdb/cp-support.c b/gdb/cp-support.c index 25c887035c..1f95253cb0 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -48,19 +48,19 @@ static unsigned int cp_find_first_component_aux (const char *name, static void demangled_name_complaint (const char *name); -/* Functions/variables related to overload resolution. */ - -static int sym_return_val_size = -1; -static int sym_return_val_index; -static struct symbol **sym_return_val; +/* Functions related to overload resolution. */ static void overload_list_add_symbol (struct symbol *sym, - const char *oload_name); + const char *oload_name, + std::vector *overload_list); -static void make_symbol_overload_list_using (const char *func_name, - const char *the_namespace); +static void add_symbol_overload_list_using + (const char *func_name, const char *the_namespace, + std::vector *overload_list); -static void make_symbol_overload_list_qualified (const char *func_name); +static void add_symbol_overload_list_qualified + (const char *func_name, + std::vector *overload_list); /* The list of "maint cplus" commands. */ @@ -1137,30 +1137,28 @@ cp_entire_prefix_len (const char *name) /* Overload resolution functions. */ /* Test to see if SYM is a symbol that we haven't seen corresponding - to a function named OLOAD_NAME. If so, add it to the current - completion list. */ + to a function named OLOAD_NAME. If so, add it to + OVERLOAD_LIST. */ static void overload_list_add_symbol (struct symbol *sym, - const char *oload_name) + const char *oload_name, + std::vector *overload_list) { - int newsize; - int i; - gdb::unique_xmalloc_ptr sym_name; - /* If there is no type information, we can't do anything, so skip. */ if (SYMBOL_TYPE (sym) == NULL) return; /* skip any symbols that we've already considered. */ - for (i = 0; i < sym_return_val_index; ++i) + for (symbol *listed_sym : *overload_list) if (strcmp (SYMBOL_LINKAGE_NAME (sym), - SYMBOL_LINKAGE_NAME (sym_return_val[i])) == 0) + SYMBOL_LINKAGE_NAME (listed_sym)) == 0) return; /* Get the demangled name without parameters */ - sym_name = cp_remove_params (SYMBOL_NATURAL_NAME (sym)); + gdb::unique_xmalloc_ptr sym_name + = cp_remove_params (SYMBOL_NATURAL_NAME (sym)); if (!sym_name) return; @@ -1168,36 +1166,22 @@ overload_list_add_symbol (struct symbol *sym, if (strcmp (sym_name.get (), oload_name) != 0) return; - /* We have a match for an overload instance, so add SYM to the - current list of overload instances */ - if (sym_return_val_index + 3 > sym_return_val_size) - { - newsize = (sym_return_val_size *= 2) * sizeof (struct symbol *); - sym_return_val = (struct symbol **) - xrealloc ((char *) sym_return_val, newsize); - } - sym_return_val[sym_return_val_index++] = sym; - sym_return_val[sym_return_val_index] = NULL; + overload_list->push_back (sym); } /* Return a null-terminated list of pointers to function symbols that are named FUNC_NAME and are visible within NAMESPACE. */ -struct symbol ** +struct std::vector make_symbol_overload_list (const char *func_name, const char *the_namespace) { - struct cleanup *old_cleanups; const char *name; + std::vector overload_list; - sym_return_val_size = 100; - sym_return_val_index = 0; - sym_return_val = XNEWVEC (struct symbol *, sym_return_val_size + 1); - sym_return_val[0] = NULL; - - old_cleanups = make_cleanup (xfree, sym_return_val); + overload_list.reserve (100); - make_symbol_overload_list_using (func_name, the_namespace); + add_symbol_overload_list_using (func_name, the_namespace, &overload_list); if (the_namespace[0] == '\0') name = func_name; @@ -1211,19 +1195,17 @@ make_symbol_overload_list (const char *func_name, name = concatenated_name; } - make_symbol_overload_list_qualified (name); - - discard_cleanups (old_cleanups); - - return sym_return_val; + add_symbol_overload_list_qualified (name, &overload_list); + return overload_list; } /* Add all symbols with a name matching NAME in BLOCK to the overload list. */ static void -make_symbol_overload_list_block (const char *name, - const struct block *block) +add_symbol_overload_list_block (const char *name, + const struct block *block, + std::vector *overload_list) { struct block_iterator iter; struct symbol *sym; @@ -1231,14 +1213,15 @@ make_symbol_overload_list_block (const char *name, lookup_name_info lookup_name (name, symbol_name_match_type::FULL); ALL_BLOCK_SYMBOLS_WITH_NAME (block, lookup_name, iter, sym) - overload_list_add_symbol (sym, name); + overload_list_add_symbol (sym, name, overload_list); } /* Adds the function FUNC_NAME from NAMESPACE to the overload set. */ static void -make_symbol_overload_list_namespace (const char *func_name, - const char *the_namespace) +add_symbol_overload_list_namespace (const char *func_name, + const char *the_namespace, + std::vector *overload_list) { const char *name; const struct block *block = NULL; @@ -1259,12 +1242,12 @@ make_symbol_overload_list_namespace (const char *func_name, /* Look in the static block. */ block = block_static_block (get_selected_block (0)); if (block) - make_symbol_overload_list_block (name, block); + add_symbol_overload_list_block (name, block, overload_list); /* Look in the global block. */ block = block_global_block (block); if (block) - make_symbol_overload_list_block (name, block); + add_symbol_overload_list_block (name, block, overload_list); } @@ -1272,8 +1255,9 @@ make_symbol_overload_list_namespace (const char *func_name, base types. */ static void -make_symbol_overload_list_adl_namespace (struct type *type, - const char *func_name) +add_symbol_overload_list_adl_namespace (struct type *type, + const char *func_name, + std::vector *overload_list) { char *the_namespace; const char *type_name; @@ -1303,7 +1287,8 @@ make_symbol_overload_list_adl_namespace (struct type *type, strncpy (the_namespace, type_name, prefix_len); the_namespace[prefix_len] = '\0'; - make_symbol_overload_list_namespace (func_name, the_namespace); + add_symbol_overload_list_namespace (func_name, the_namespace, + overload_list); } /* Check public base type */ @@ -1311,28 +1296,23 @@ make_symbol_overload_list_adl_namespace (struct type *type, for (i = 0; i < TYPE_N_BASECLASSES (type); i++) { if (BASETYPE_VIA_PUBLIC (type, i)) - make_symbol_overload_list_adl_namespace (TYPE_BASECLASS (type, - i), - func_name); + add_symbol_overload_list_adl_namespace (TYPE_BASECLASS (type, i), + func_name, + overload_list); } } -/* Adds the overload list overload candidates for FUNC_NAME found - through argument dependent lookup. */ +/* Adds to OVERLOAD_LIST the overload list overload candidates for + FUNC_NAME found through argument dependent lookup. */ -struct symbol ** -make_symbol_overload_list_adl (struct type **arg_types, int nargs, - const char *func_name) +void +add_symbol_overload_list_adl (gdb::array_view arg_types, + const char *func_name, + std::vector *overload_list) { - int i; - - gdb_assert (sym_return_val_size != -1); - - for (i = 1; i <= nargs; i++) - make_symbol_overload_list_adl_namespace (arg_types[i - 1], - func_name); - - return sym_return_val; + for (type *arg_type : arg_types) + add_symbol_overload_list_adl_namespace (arg_type, func_name, + overload_list); } /* This applies the using directives to add namespaces to search in, @@ -1341,8 +1321,9 @@ make_symbol_overload_list_adl (struct type **arg_types, int nargs, make_symbol_overload_list. */ static void -make_symbol_overload_list_using (const char *func_name, - const char *the_namespace) +add_symbol_overload_list_using (const char *func_name, + const char *the_namespace, + std::vector *overload_list) { struct using_direct *current; const struct block *block; @@ -1374,13 +1355,15 @@ make_symbol_overload_list_using (const char *func_name, scoped_restore reset_directive_searched = make_scoped_restore (¤t->searched, 1); - make_symbol_overload_list_using (func_name, - current->import_src); + add_symbol_overload_list_using (func_name, + current->import_src, + overload_list); } } /* Now, add names for this namespace. */ - make_symbol_overload_list_namespace (func_name, the_namespace); + add_symbol_overload_list_namespace (func_name, the_namespace, + overload_list); } /* This does the bulk of the work of finding overloaded symbols. @@ -1388,7 +1371,8 @@ make_symbol_overload_list_using (const char *func_name, (possibly including namespace info). */ static void -make_symbol_overload_list_qualified (const char *func_name) +add_symbol_overload_list_qualified (const char *func_name, + std::vector *overload_list) { struct compunit_symtab *cust; struct objfile *objfile; @@ -1407,7 +1391,7 @@ make_symbol_overload_list_qualified (const char *func_name) complete on local vars. */ for (b = get_selected_block (0); b != NULL; b = BLOCK_SUPERBLOCK (b)) - make_symbol_overload_list_block (func_name, b); + add_symbol_overload_list_block (func_name, b, overload_list); surrounding_static_block = block_static_block (get_selected_block (0)); @@ -1418,7 +1402,7 @@ make_symbol_overload_list_qualified (const char *func_name) { QUIT; b = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (cust), GLOBAL_BLOCK); - make_symbol_overload_list_block (func_name, b); + add_symbol_overload_list_block (func_name, b, overload_list); } ALL_COMPUNITS (objfile, cust) @@ -1428,7 +1412,7 @@ make_symbol_overload_list_qualified (const char *func_name) /* Don't do this block twice. */ if (b == surrounding_static_block) continue; - make_symbol_overload_list_block (func_name, b); + add_symbol_overload_list_block (func_name, b, overload_list); } } diff --git a/gdb/cp-support.h b/gdb/cp-support.h index 4e26921595..0402df02d3 100644 --- a/gdb/cp-support.h +++ b/gdb/cp-support.h @@ -28,6 +28,8 @@ #include "vec.h" #include "gdb_vecs.h" #include "gdb_obstack.h" +#include "common/array-view.h" +#include /* Opaque declarations. */ @@ -107,12 +109,13 @@ extern gdb::unique_xmalloc_ptr cp_remove_params extern gdb::unique_xmalloc_ptr cp_remove_params_if_any (const char *demangled_name, bool completion_mode); -extern struct symbol **make_symbol_overload_list (const char *, - const char *); +extern std::vector make_symbol_overload_list (const char *, + const char *); -extern struct symbol **make_symbol_overload_list_adl (struct type **arg_types, - int nargs, - const char *func_name); +extern void add_symbol_overload_list_adl + (gdb::array_view arg_types, + const char *func_name, + std::vector *overload_list); extern struct type *cp_lookup_rtti_type (const char *name, struct block *block); diff --git a/gdb/valops.c b/gdb/valops.c index f0e53a7ce9..7fbedd77bb 100644 --- a/gdb/valops.c +++ b/gdb/valops.c @@ -56,13 +56,13 @@ static struct value *search_struct_method (const char *, struct value **, static int find_oload_champ_namespace (gdb::array_view args, const char *, const char *, - struct symbol ***, + std::vector *oload_syms, struct badness_vector **, const int no_adl); static int find_oload_champ_namespace_loop (gdb::array_view args, const char *, const char *, - int, struct symbol ***, + int, std::vector *oload_syms, struct badness_vector **, int *, const int no_adl); @@ -2517,7 +2517,7 @@ find_overload_match (gdb::array_view args, /* 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; + std::vector oload_syms; /* For xmethods, the vector of xmethod workers. */ std::vector xm_worker_vec; /* Number of overloaded instances being considered. */ @@ -2717,7 +2717,6 @@ find_overload_match (gdb::array_view args, func_match_quality = classify_oload_match (func_badness, args.size (), 0); - make_cleanup (xfree, oload_syms); make_cleanup (xfree, func_badness); } @@ -2857,7 +2856,7 @@ static int find_oload_champ_namespace (gdb::array_view args, const char *func_name, const char *qualified_name, - struct symbol ***oload_syms, + std::vector *oload_syms, struct badness_vector **oload_champ_bv, const int no_adl) { @@ -2887,17 +2886,15 @@ find_oload_champ_namespace_loop (gdb::array_view args, const char *func_name, const char *qualified_name, int namespace_len, - struct symbol ***oload_syms, + std::vector *oload_syms, struct badness_vector **oload_champ_bv, int *oload_champ, const int no_adl) { int next_namespace_len = namespace_len; int searched_deeper = 0; - int num_fns = 0; struct cleanup *old_cleanups; int new_oload_champ; - struct symbol **new_oload_syms; struct badness_vector *new_oload_champ_bv; char *new_namespace; @@ -2910,7 +2907,6 @@ find_oload_champ_namespace_loop (gdb::array_view args, cp_find_first_component (qualified_name + next_namespace_len); /* Initialize these to values that can safely be xfree'd. */ - *oload_syms = NULL; *oload_champ_bv = NULL; /* First, see if we have a deeper namespace we can search in. @@ -2938,13 +2934,13 @@ find_oload_champ_namespace_loop (gdb::array_view args, because this overload mechanism only gets called if there's a function symbol to start off with.) */ - old_cleanups = make_cleanup (xfree, *oload_syms); - make_cleanup (xfree, *oload_champ_bv); + old_cleanups = make_cleanup (xfree, *oload_champ_bv); new_namespace = (char *) alloca (namespace_len + 1); strncpy (new_namespace, qualified_name, namespace_len); new_namespace[namespace_len] = '\0'; - new_oload_syms = make_symbol_overload_list (func_name, - new_namespace); + + std::vector new_oload_syms + = make_symbol_overload_list (func_name, new_namespace); /* If we have reached the deepest level perform argument determined lookup. */ @@ -2958,14 +2954,12 @@ find_oload_champ_namespace_loop (gdb::array_view args, alloca (args.size () * (sizeof (struct type *))); for (ix = 0; ix < args.size (); ix++) arg_types[ix] = value_type (args[ix]); - make_symbol_overload_list_adl (arg_types, args.size (), func_name); + add_symbol_overload_list_adl ({arg_types, args.size ()}, func_name, + &new_oload_syms); } - while (new_oload_syms[num_fns]) - ++num_fns; - - new_oload_champ = find_oload_champ (args, num_fns, - NULL, NULL, new_oload_syms, + new_oload_champ = find_oload_champ (args, new_oload_syms.size (), + NULL, NULL, new_oload_syms.data (), &new_oload_champ_bv); /* Case 1: We found a good match. Free earlier matches (if any), @@ -2978,7 +2972,7 @@ find_oload_champ_namespace_loop (gdb::array_view args, if (new_oload_champ != -1 && classify_oload_match (new_oload_champ_bv, args.size (), 0) == STANDARD) { - *oload_syms = new_oload_syms; + *oload_syms = std::move (new_oload_syms); *oload_champ = new_oload_champ; *oload_champ_bv = new_oload_champ_bv; do_cleanups (old_cleanups); @@ -2986,14 +2980,13 @@ find_oload_champ_namespace_loop (gdb::array_view args, } else if (searched_deeper) { - xfree (new_oload_syms); xfree (new_oload_champ_bv); discard_cleanups (old_cleanups); return 0; } else { - *oload_syms = new_oload_syms; + *oload_syms = std::move (new_oload_syms); *oload_champ = new_oload_champ; *oload_champ_bv = new_oload_champ_bv; do_cleanups (old_cleanups);