[2/2] Avoid copying in lookup_name_info

Message ID 20200320204935.19509-3-tromey@adacore.com
State Superseded
Headers show
Series
  • Avoid copying in name lookup
Related show

Commit Message

Tom Tromey March 20, 2020, 8:49 p.m.
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  <tromey@adacore.com>

	* symtab.h (class lookup_name_info) <lookup_name_info>: Change
	"name" parameter to rvalue reference.  Initialize m_name_holder.
	<lookup_name_info>: New overloads.
	<name>: Return gdb::string_view.
	<c_str>: New method.
	<make_ignore_params>: Update.
	<search_name_hash>: Update.
	<language_lookup_name>: Return const char *.
	<m_name_holder>: Rename from m_name.
	<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(-)

-- 
2.21.1

Comments

Victor Collod via Gdb-patches March 23, 2020, 5:51 p.m. | #1
On Fri, Mar 20, 2020 at 3:49 PM Tom Tromey <tromey@adacore.com> wrote:
>

> 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  <tromey@adacore.com>

>

>         * symtab.h (class lookup_name_info) <lookup_name_info>: Change

>         "name" parameter to rvalue reference.  Initialize m_name_holder.

>         <lookup_name_info>: New overloads.

>         <name>: Return gdb::string_view.

>         <c_str>: New method.

>         <make_ignore_params>: Update.

>         <search_name_hash>: Update.

>         <language_lookup_name>: Return const char *.

>         <m_name_holder>: Rename from m_name.

>         <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));


You can just use symbol_search_name == name_view, since
gdb::string_view has an overloaded operator==. That way you don't need
to call c_str() here.

>    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<char> 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

>
Tom Tromey March 31, 2020, 1:28 p.m. | #2
>>>>> "Christian" == Christian Biesinger <cbiesinger@google.com> writes:


>> 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));


Christian> You can just use symbol_search_name == name_view, since
Christian> gdb::string_view has an overloaded operator==. That way you don't need
Christian> to call c_str() here.

Thanks, I made this change.  I removed "cmp" as well, so now it looks
like:

  if (lookup_name.completion_mode ()
      ? (strncmp (symbol_search_name, name_view.data (),
		  name_view.size ()) == 0)
      : symbol_search_name == name_view)

C++20 has a starts_with method on string view, but we don't have that
yet.

Tom
Victor Collod via Gdb-patches March 31, 2020, 6:18 p.m. | #3
On 3/20/20 8:49 PM, Tom Tromey wrote:

>    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,


I think it would be better for this overload to take a
gdb::string_view instead of a const std::string& .  That
way, it still works if you only have a string_view handy.
As is, there's no way to created a lookup_name_info starting
with a string_view without copying.  WDYT?

And if we do that, do we have any case where we really need the
"std::string &&" overload?  You mention that normally copying
isn't needed -- but when is copying needed?  I.e., do we have
cases where the lookup_name_info object doesn't have a lifetime greater
than the original string?  If there isn't, then we could also get
rid of name_holder, and avoid having to remember that different
ctors behave differently.

Thanks,
Pedro Alves
Tom Tromey March 31, 2020, 7:11 p.m. | #4
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> I think it would be better for this overload to take a
Pedro> gdb::string_view instead of a const std::string& .  That
Pedro> way, it still works if you only have a string_view handy.
Pedro> As is, there's no way to created a lookup_name_info starting
Pedro> with a string_view without copying.  WDYT?

The issue is that there's a trick in the implementation:

  /* 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 ();
  }

This is needed in some spots.

However, if we take a string_view at construction, then we'd be
violating the string_view contract, or we'd need to make a copy -- but
the former is bad, and the latter is the goal the patch.

Pedro> And if we do that, do we have any case where we really need the
Pedro> "std::string &&" overload?

Indeed we don't seem to need this.

Tom
Tom Tromey March 31, 2020, 7:15 p.m. | #5
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:


Pedro> And if we do that, do we have any case where we really need the
Pedro> "std::string &&" overload?

Tom> Indeed we don't seem to need this.

Actually, I am wrong, there is a spot in ada-lang.c:

      lookup_name_info name1 (std::string ("<_ada_") + name + '>',
			      symbol_name_match_type::FULL);

What's concerning is that this compiles even without the && overload,
presumably by just violating the lifetime requirement of lookup_name_info.

So, I think leaving the && version in-place is best.

Tom
Victor Collod via Gdb-patches March 31, 2020, 7:23 p.m. | #6
On 3/31/20 8:11 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

> 

> Pedro> I think it would be better for this overload to take a

> Pedro> gdb::string_view instead of a const std::string& .  That

> Pedro> way, it still works if you only have a string_view handy.

> Pedro> As is, there's no way to created a lookup_name_info starting

> Pedro> with a string_view without copying.  WDYT?

> 

> The issue is that there's a trick in the implementation:

> 

>   /* 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 ();

>   }

> 

> This is needed in some spots.

> 

> However, if we take a string_view at construction, then we'd be

> violating the string_view contract, or we'd need to make a copy -- but

> the former is bad, and the latter is the goal the patch.


I see.  I agree with leaving it as is then.

Thanks,
Pedro Alves
Victor Collod via Gdb-patches March 31, 2020, 7:28 p.m. | #7
On 3/31/20 8:15 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

> 

> Pedro> And if we do that, do we have any case where we really need the

> Pedro> "std::string &&" overload?

> 

> Tom> Indeed we don't seem to need this.

> 

> Actually, I am wrong, there is a spot in ada-lang.c:

> 

>       lookup_name_info name1 (std::string ("<_ada_") + name + '>',

> 			      symbol_name_match_type::FULL);

> 

> What's concerning is that this compiles even without the && overload,

> presumably by just violating the lifetime requirement of lookup_name_info.

> 

> So, I think leaving the && version in-place is best.


Alternatively, you could delete the && version, like:

  lookup_name_info (std::string &&name, ......) = delete;

I think that would catch that case, making it fail to compile,
forcing us to write:

 std::string str = std::string ("<_ada_") + name + '>';
 lookup_name_info name1 (str, symbol_name_match_type::FULL);

I think I'd slightly prefer that for making lookup_name_info's
contract easier to grok, though I'll leave it to you.

Thanks,
Pedro Alves
Tom Tromey March 31, 2020, 7:35 p.m. | #8
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> Alternatively, you could delete the && version, like:

Pedro>   lookup_name_info (std::string &&name, ......) = delete;

Pedro> I think that would catch that case, making it fail to compile,
Pedro> forcing us to write:

Thanks for the idea.  I'll do this.

Tom

Patch

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<char> 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