From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [IPv6:2620:20:4000:0:a9e:1ff:fe9b:1d1]) by sourceware.org (Postfix) with ESMTP id 90D0B38A1031 for ; Fri, 20 Mar 2020 20:49:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 90D0B38A1031 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 64F1B56165; Fri, 20 Mar 2020 16:49:37 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id D2ryTKujhfFz; Fri, 20 Mar 2020 16:49:37 -0400 (EDT) Received: from murgatroyd.Home (97-118-117-21.hlrn.qwest.net [97.118.117.21]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPSA id 2001B56162; Fri, 20 Mar 2020 16:49:37 -0400 (EDT) From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH 2/2] Avoid copying in lookup_name_info Date: Fri, 20 Mar 2020 14:49:35 -0600 Message-Id: <20200320204935.19509-3-tromey@adacore.com> X-Mailer: git-send-email 2.21.1 In-Reply-To: <20200320204935.19509-1-tromey@adacore.com> References: <20200320204935.19509-1-tromey@adacore.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-26.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, T_FILL_THIS_FORM_SHORT autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Mar 2020 20:49:39 -0000 lookup_name_info always copies the name that is passed in. However, normally a copy is not needed. This patch changes this class to avoid copying. This required changing the "name" method to return something else; I chose a gdb::string_view, to avoid excessive calls to strlen in the code using the lookup_name_info. However, as this class does not allow an arbitrary string_view, I've also added a c_str method that guarantees a \0-terminated result -- a pedantic difference but one that respects the string_view contract, IMO. gdb/ChangeLog 2020-03-20 Tom Tromey * symtab.h (class lookup_name_info) : Change "name" parameter to rvalue reference. Initialize m_name_holder. : New overloads. : Return gdb::string_view. : New method. : Update. : Update. : Return const char *. : Rename from m_name. : New member. * symtab.c (demangle_for_lookup_info::demangle_for_lookup_info) (demangle_for_lookup_info::demangle_for_lookup_info): Update. (lookup_name_info::match_any): Update. * psymtab.c (match_partial_symbol, lookup_partial_symbol): Update. * minsyms.c (linkage_name_str): Update. * language.c (default_symbol_name_matcher): Update. * dwarf2/read.c (mapped_index_base::find_name_components_bounds): Update. * ada-lang.c (ada_fold_name): Change parameter to string_view. (ada_lookup_name_info::ada_lookup_name_info): Update. (literal_symbol_name_matcher): Update. --- gdb/ChangeLog | 25 ++++++++++++++++++++ gdb/ada-lang.c | 29 ++++++++++++----------- gdb/dwarf2/read.c | 2 +- gdb/language.c | 4 ++-- gdb/minsyms.c | 2 +- gdb/psymtab.c | 5 ++-- gdb/symtab.c | 8 +++---- gdb/symtab.h | 59 +++++++++++++++++++++++++++++++++++++---------- 8 files changed, 98 insertions(+), 36 deletions(-) diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 2822d40c8cd..c38ed1fca0e 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -1016,17 +1016,17 @@ ada_encode (const char *decoded) to next call. */ static char * -ada_fold_name (const char *name) +ada_fold_name (gdb::string_view name) { static char *fold_buffer = NULL; static size_t fold_buffer_size = 0; - int len = strlen (name); + int len = name.size (); GROW_VECT (fold_buffer, fold_buffer_size, len + 1); if (name[0] == '\'') { - strncpy (fold_buffer, name + 1, len - 2); + strncpy (fold_buffer, name.data () + 1, len - 2); fold_buffer[len - 2] = '\000'; } else @@ -13953,14 +13953,16 @@ do_exact_match (const char *symbol_search_name, ada_lookup_name_info::ada_lookup_name_info (const lookup_name_info &lookup_name) { - const std::string &user_name = lookup_name.name (); + gdb::string_view user_name = lookup_name.name (); if (user_name[0] == '<') { if (user_name.back () == '>') - m_encoded_name = user_name.substr (1, user_name.size () - 2); + m_encoded_name + = user_name.substr (1, user_name.size () - 2).to_string (); else - m_encoded_name = user_name.substr (1, user_name.size () - 1); + m_encoded_name + = user_name.substr (1, user_name.size () - 1).to_string (); m_encoded_p = true; m_verbatim_p = true; m_wild_match_p = false; @@ -13970,19 +13972,19 @@ ada_lookup_name_info::ada_lookup_name_info (const lookup_name_info &lookup_name) { m_verbatim_p = false; - m_encoded_p = user_name.find ("__") != std::string::npos; + m_encoded_p = user_name.find ("__") != gdb::string_view::npos; if (!m_encoded_p) { - const char *folded = ada_fold_name (user_name.c_str ()); + const char *folded = ada_fold_name (user_name); const char *encoded = ada_encode_1 (folded, false); if (encoded != NULL) m_encoded_name = encoded; else - m_encoded_name = user_name; + m_encoded_name = user_name.to_string (); } else - m_encoded_name = user_name; + m_encoded_name = user_name.to_string (); /* Handle the 'package Standard' special case. See description of m_standard_p. */ @@ -14029,11 +14031,12 @@ literal_symbol_name_matcher (const char *symbol_search_name, const lookup_name_info &lookup_name, completion_match_result *comp_match_res) { - const std::string &name = lookup_name.name (); + gdb::string_view name_view = lookup_name.name (); + const char *name = lookup_name.c_str (); int cmp = (lookup_name.completion_mode () - ? strncmp (symbol_search_name, name.c_str (), name.size ()) - : strcmp (symbol_search_name, name.c_str ())); + ? strncmp (symbol_search_name, name, name_view.size ()) + : strcmp (symbol_search_name, name)); if (cmp == 0) { if (comp_match_res != NULL) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 0e879e08a0c..cdccd04c54a 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -3774,7 +3774,7 @@ mapped_index_base::find_name_components_bounds = this->name_components_casing == case_sensitive_on ? strcmp : strcasecmp; const char *lang_name - = lookup_name_without_params.language_lookup_name (lang).c_str (); + = lookup_name_without_params.language_lookup_name (lang); /* Comparison function object for lower_bound that matches against a given symbol name. */ diff --git a/gdb/language.c b/gdb/language.c index 454c6dc45a7..c13fd1a406a 100644 --- a/gdb/language.c +++ b/gdb/language.c @@ -699,14 +699,14 @@ default_symbol_name_matcher (const char *symbol_search_name, const lookup_name_info &lookup_name, completion_match_result *comp_match_res) { - const std::string &name = lookup_name.name (); + gdb::string_view name = lookup_name.name (); completion_match_for_lcd *match_for_lcd = (comp_match_res != NULL ? &comp_match_res->match_for_lcd : NULL); strncmp_iw_mode mode = (lookup_name.completion_mode () ? strncmp_iw_mode::NORMAL : strncmp_iw_mode::MATCH_PARAMS); - if (strncmp_iw_with_mode (symbol_search_name, name.c_str (), name.size (), + if (strncmp_iw_with_mode (symbol_search_name, name.data (), name.size (), mode, language_minimal, match_for_lcd) == 0) { if (comp_match_res != NULL) diff --git a/gdb/minsyms.c b/gdb/minsyms.c index e238355dc11..d2ac8172eea 100644 --- a/gdb/minsyms.c +++ b/gdb/minsyms.c @@ -467,7 +467,7 @@ linkage_name_str (const lookup_name_info &lookup_name) if (current_language->la_language == language_ada) return lookup_name.ada ().lookup_name ().c_str (); - return lookup_name.name ().c_str (); + return lookup_name.c_str (); } /* See minsyms.h. */ diff --git a/gdb/psymtab.c b/gdb/psymtab.c index 34b9741e966..7ab861632eb 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -612,8 +612,7 @@ match_partial_symbol (struct objfile *objfile, gdb_assert (center < top); enum language lang = (*center)->ginfo.language (); - const char *lang_ln - = name.language_lookup_name (lang).c_str (); + const char *lang_ln = name.language_lookup_name (lang); if (ordered_compare ((*center)->ginfo.search_name (), lang_ln) >= 0) @@ -692,7 +691,7 @@ lookup_partial_symbol (struct objfile *objfile, internal_error (__FILE__, __LINE__, _("failed internal consistency check")); if (strcmp_iw_ordered ((*center)->ginfo.search_name (), - lookup_name.name ().c_str ()) >= 0) + lookup_name.c_str ()) >= 0) { top = center; } diff --git a/gdb/symtab.c b/gdb/symtab.c index f300d759e03..680280105f5 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -1790,7 +1790,7 @@ demangle_for_lookup_info::demangle_for_lookup_info if (lookup_name.ignore_parameters () && lang == language_cplus) { gdb::unique_xmalloc_ptr without_params - = cp_remove_params_if_any (lookup_name.name ().c_str (), + = cp_remove_params_if_any (lookup_name.c_str (), lookup_name.completion_mode ()); if (without_params != NULL) @@ -1803,9 +1803,9 @@ demangle_for_lookup_info::demangle_for_lookup_info } if (lookup_name.match_type () == symbol_name_match_type::SEARCH_NAME) - m_demangled_name = lookup_name.name (); + m_demangled_name = lookup_name.c_str (); else - m_demangled_name = demangle_for_lookup (lookup_name.name ().c_str (), + m_demangled_name = demangle_for_lookup (lookup_name.c_str (), lang, storage); } @@ -1816,7 +1816,7 @@ lookup_name_info::match_any () { /* Lookup any symbol that "" would complete. I.e., this matches all symbol names. */ - static const lookup_name_info lookup_name ({}, symbol_name_match_type::FULL, + static const lookup_name_info lookup_name ("", symbol_name_match_type::FULL, true); return lookup_name; diff --git a/gdb/symtab.h b/gdb/symtab.h index 771b5ec5bf7..c80acc3a68e 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -186,28 +186,62 @@ class lookup_name_info final { public: /* Create a new object. */ - lookup_name_info (std::string name, + lookup_name_info (std::string &&name, symbol_name_match_type match_type, bool completion_mode = false, bool ignore_parameters = false) : m_match_type (match_type), m_completion_mode (completion_mode), m_ignore_parameters (ignore_parameters), - m_name (std::move (name)) + m_name_holder (std::move (name)), + m_name (m_name_holder.c_str ()) + {} + + /* This overload requires that NAME have a lifetime at least as long + as the lifetime of this object. */ + lookup_name_info (const std::string &name, + symbol_name_match_type match_type, + bool completion_mode = false, + bool ignore_parameters = false) + : m_match_type (match_type), + m_completion_mode (completion_mode), + m_ignore_parameters (ignore_parameters), + m_name (name) + {} + + /* This overload requires that NAME have a lifetime at least as long + as the lifetime of this object. */ + lookup_name_info (const char *name, + symbol_name_match_type match_type, + bool completion_mode = false, + bool ignore_parameters = false) + : m_match_type (match_type), + m_completion_mode (completion_mode), + m_ignore_parameters (ignore_parameters), + m_name (name) {} /* Getters. See description of each corresponding field. */ symbol_name_match_type match_type () const { return m_match_type; } bool completion_mode () const { return m_completion_mode; } - const std::string &name () const { return m_name; } + gdb::string_view name () const { return m_name; } const bool ignore_parameters () const { return m_ignore_parameters; } + /* Like the "name" method but guarantees that the returned string is + \0-terminated. */ + const char *c_str () const + { + /* Actually this is always guaranteed due to how the class is + constructed. */ + return m_name.data (); + } + /* Return a version of this lookup name that is usable with comparisons against symbols have no parameter info, such as psymbols and GDB index symbols. */ lookup_name_info make_ignore_params () const { - return lookup_name_info (m_name, m_match_type, m_completion_mode, + return lookup_name_info (c_str (), m_match_type, m_completion_mode, true /* ignore params */); } @@ -218,27 +252,27 @@ class lookup_name_info final if (!m_demangled_hashes_p[lang]) { m_demangled_hashes[lang] - = ::search_name_hash (lang, language_lookup_name (lang).c_str ()); + = ::search_name_hash (lang, language_lookup_name (lang)); m_demangled_hashes_p[lang] = true; } return m_demangled_hashes[lang]; } /* Get the search name for searches in language LANG. */ - const std::string &language_lookup_name (language lang) const + const char *language_lookup_name (language lang) const { switch (lang) { case language_ada: - return ada ().lookup_name (); + return ada ().lookup_name ().c_str (); case language_cplus: - return cplus ().lookup_name (); + return cplus ().lookup_name ().c_str (); case language_d: - return d ().lookup_name (); + return d ().lookup_name ().c_str (); case language_go: - return go ().lookup_name (); + return go ().lookup_name ().c_str (); default: - return m_name; + return m_name.data (); } } @@ -287,7 +321,8 @@ class lookup_name_info final symbol_name_match_type m_match_type; bool m_completion_mode; bool m_ignore_parameters; - std::string m_name; + std::string m_name_holder; + gdb::string_view m_name; /* Language-specific info. These fields are filled lazily the first time a lookup is done in the corresponding language. They're -- 2.21.1