From patchwork Fri Aug 17 18:04:07 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Seitz X-Patchwork-Id: 28958 Received: (qmail 48149 invoked by alias); 17 Aug 2018 18:04:11 -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 48129 invoked by uid 89); 17 Aug 2018 18:04:11 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, 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; Fri, 17 Aug 2018 18:04:09 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3267D3082E08; Fri, 17 Aug 2018 18:04:08 +0000 (UTC) Received: from theo.uglyboxes.com (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EBD10608E6; Fri, 17 Aug 2018 18:04:07 +0000 (UTC) Subject: Re: [PATCH 5/9] Change decode_compound_collector to use std::vector To: Tom Tromey Cc: gdb-patches@sourceware.org References: <20180810232534.481-1-keiths@redhat.com> <20180810232534.481-6-keiths@redhat.com> <87pnypt086.fsf@tromey.com> From: Keith Seitz Message-ID: <9675ef7a-9d3c-5dfb-3f56-e43d80eb02d5@redhat.com> Date: Fri, 17 Aug 2018 11:04:07 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <87pnypt086.fsf@tromey.com> X-IsSubscribed: yes On 08/11/2018 08:02 AM, Tom Tromey wrote: >>>>>> "Keith" == Keith Seitz writes: > > Keith> This patch changes decode_compound_collector to use std::vector instead of > Keith> VEC, eliminating a cleanup in the process. > > Keith> - /* Releases ownership of the collected symbols and returns them. */ > Keith> - VEC (symbolp) *release_symbols () > Keith> + /* Return all symbols collected. */ > Keith> + std::vector release_symbols () > Keith> { > Keith> - VEC (symbolp) *res = m_symbols; > Keith> - m_symbols = NULL; > Keith> - return res; > Keith> + return m_symbols; > Keith> } > > Won't this end up copying m_symbols? Yup. Forgot to move the return value (which would also clear/reset m_symbols as expected today). This patch has one additional change over v1: the final qsort is replaced with std::sort and the temporary std_compare_symbol function replaces the qsort-oriented compare_symbol function (mentioned in a previous patch email). Keith Subject: [PATCH] Change decode_compound_collector to use std::vector This patch changes decode_compound_collector to use std::vector instead of VEC, eliminating a cleanup in the process. gdb/ChangeLog: * linespec.c (decode_compound_collector::decode_compound_collector): Remove initialization for `m_symtabs'. (decode_compound_collector::release_symbols): Change return type to std::vector. Update all callers. (class decode_compound_collector) : Change type to std::vector. (lookup_prefix_sym): Change return type to std::vector. Update all callers. (compare_symbols): Remove. (std_compare_symbols): Rename to `compare_symbols'. (find_method): Change `sym_classes' parameter to std::vector. Update all callers. Use std::sort to sort sym_classes. (find_linespec_symbols): Remove cleanup. --- gdb/ChangeLog | 16 +++++++++++ gdb/linespec.c | 85 +++++++++++++++------------------------------------------- 2 files changed, 37 insertions(+), 64 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index a94e1cdd68..d81e6f0fbd 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,21 @@ YYYY-MM-DD Keith Seitz + * linespec.c (decode_compound_collector::decode_compound_collector): + Remove initialization for `m_symtabs'. + (decode_compound_collector::release_symbols): Change return type + to std::vector. Update all callers. + (class decode_compound_collector) : Change type to + std::vector. + (lookup_prefix_sym): Change return type to std::vector. Update all + callers. + (compare_symbols): Remove. + (std_compare_symbols): Rename to `compare_symbols'. + (find_method): Change `sym_classes' parameter to std::vector. + Update all callers. Use std::sort to sort sym_classes. + (find_linespec_symbols): Remove cleanup. + +YYYY-MM-DD Keith Seitz + * linespec.c (struct linespec) : Change type to std::vector. Update all users. (convert_linespec_to_sals): Use std::sort to sort minimal symbols. diff --git a/gdb/linespec.c b/gdb/linespec.c index e7e19cab67..926fbb868f 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -400,9 +400,7 @@ static void minsym_found (struct linespec_state *self, struct objfile *objfile, struct minimal_symbol *msymbol, std::vector *result); -static int compare_symbols (const void *a, const void *b); -static bool std_compare_symbols (const struct symbol *a, - const struct symbol *b); +static bool compare_symbols (const struct symbol *a, const struct symbol *b); static bool compare_msymbols (const bound_minimal_symbol &a, const bound_minimal_symbol &b); @@ -2266,7 +2264,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls) to each other. */ std::sort (ls->function_symbols->begin (), ls->function_symbols->end (), - std_compare_symbols); + compare_symbols); for (const auto &sym : *ls->function_symbols) { @@ -3501,7 +3499,6 @@ class decode_compound_collector { public: decode_compound_collector () - : m_symbols (NULL) { m_unique_syms = htab_create_alloc (1, htab_hash_pointer, htab_eq_pointer, NULL, @@ -3514,12 +3511,10 @@ public: htab_delete (m_unique_syms); } - /* Releases ownership of the collected symbols and returns them. */ - VEC (symbolp) *release_symbols () + /* Return all symbols collected. */ + std::vector release_symbols () { - VEC (symbolp) *res = m_symbols; - m_symbols = NULL; - return res; + return std::move (m_symbols); } /* Callable as a symbol_found_callback_ftype callback. */ @@ -3531,7 +3526,7 @@ private: htab_t m_unique_syms; /* The result vector. */ - VEC (symbolp) *m_symbols; + std::vector m_symbols; }; bool @@ -3554,7 +3549,7 @@ decode_compound_collector::operator () (symbol *sym) if (!*slot) { *slot = sym; - VEC_safe_push (symbolp, m_symbols, sym); + m_symbols.push_back (sym); } return true; /* Continue iterating. */ @@ -3564,7 +3559,7 @@ decode_compound_collector::operator () (symbol *sym) /* Return any symbols corresponding to CLASS_NAME in FILE_SYMTABS. */ -static VEC (symbolp) * +static std::vector lookup_prefix_sym (struct linespec_state *state, std::vector *file_symtabs, const char *class_name) @@ -3603,7 +3598,7 @@ lookup_prefix_sym (struct linespec_state *state, symbols with the same program space end up next to each other. */ static bool -std_compare_symbols (const struct symbol *a, const struct symbol *b) +compare_symbols (const struct symbol *a, const struct symbol *b) { uintptr_t uia, uib; @@ -3624,36 +3619,6 @@ std_compare_symbols (const struct symbol *a, const struct symbol *b) return false; } -/* A qsort comparison function for symbols. The resulting order does - not actually matter; we just need to be able to sort them so that - symbols with the same program space end up next to each other. */ - -static int -compare_symbols (const void *a, const void *b) -{ - struct symbol * const *sa = (struct symbol * const*) a; - struct symbol * const *sb = (struct symbol * const*) b; - uintptr_t uia, uib; - - uia = (uintptr_t) SYMTAB_PSPACE (symbol_symtab (*sa)); - uib = (uintptr_t) SYMTAB_PSPACE (symbol_symtab (*sb)); - - if (uia < uib) - return -1; - if (uia > uib) - return 1; - - uia = (uintptr_t) *sa; - uib = (uintptr_t) *sb; - - if (uia < uib) - return -1; - if (uia > uib) - return 1; - - return 0; -} - /* Like compare_symbols but for minimal symbols. */ static bool @@ -3723,11 +3688,9 @@ find_superclass_methods (std::vector &&superclasses, static void find_method (struct linespec_state *self, std::vector *file_symtabs, const char *class_name, const char *method_name, - VEC (symbolp) *sym_classes, std::vector *symbols, + std::vector *sym_classes, std::vector *symbols, std::vector *minsyms) { - struct symbol *sym; - int ix; size_t last_result_len; std::vector superclass_vec; std::vector result_names; @@ -3735,10 +3698,8 @@ find_method (struct linespec_state *self, std::vector *file_symtabs, /* Sort symbols so that symbols with the same program space are next to each other. */ - qsort (VEC_address (symbolp, sym_classes), - VEC_length (symbolp, sym_classes), - sizeof (symbolp), - compare_symbols); + std::sort (sym_classes->begin (), sym_classes->end (), + compare_symbols); info.state = self; info.file_symtabs = file_symtabs; @@ -3755,7 +3716,8 @@ find_method (struct linespec_state *self, std::vector *file_symtabs, because we collect data across the program space before deciding what to do. */ last_result_len = 0; - for (ix = 0; VEC_iterate (symbolp, sym_classes, ix, sym); ++ix) + unsigned int ix = 0; + for (const auto &sym : *sym_classes) { struct type *t; struct program_space *pspace; @@ -3771,10 +3733,9 @@ find_method (struct linespec_state *self, std::vector *file_symtabs, /* Handle all items from a single program space at once; and be sure not to miss the last batch. */ - if (ix == VEC_length (symbolp, sym_classes) - 1 + if (ix == sym_classes->size () - 1 || (pspace - != SYMTAB_PSPACE (symbol_symtab (VEC_index (symbolp, sym_classes, - ix + 1))))) + != SYMTAB_PSPACE (symbol_symtab (sym_classes->at (ix + 1))))) { /* If we did not find a direct implementation anywhere in this program space, consider superclasses. */ @@ -3790,6 +3751,7 @@ find_method (struct linespec_state *self, std::vector *file_symtabs, superclass_vec.clear (); last_result_len = result_names.size (); + ++ix; } } @@ -3976,7 +3938,6 @@ find_linespec_symbols (struct linespec_state *state, { std::string klass, method; const char *last, *p, *scope_op; - VEC (symbolp) *classes; /* See if we can find a scope operator and break this symbol name into namespaces${SCOPE_OPERATOR}class_name and method_name. */ @@ -4005,18 +3966,16 @@ find_linespec_symbols (struct linespec_state *state, method = last; /* Find a list of classes named KLASS. */ - classes = lookup_prefix_sym (state, file_symtabs, klass.c_str ()); - struct cleanup *old_chain - = make_cleanup (VEC_cleanup (symbolp), &classes); - - if (!VEC_empty (symbolp, classes)) + std::vector classes + = lookup_prefix_sym (state, file_symtabs, klass.c_str ()); + if (!classes.empty ()) { /* Now locate a list of suitable methods named METHOD. */ TRY { find_method (state, file_symtabs, klass.c_str (), method.c_str (), - classes, symbols, minsyms); + &classes, symbols, minsyms); } /* If successful, we're done. If NOT_FOUND_ERROR @@ -4028,8 +3987,6 @@ find_linespec_symbols (struct linespec_state *state, } END_CATCH } - - do_cleanups (old_chain); } }