Re: [PATCH 2/2] Avoid copying in lookup_name_info

[Pedro Alves <palves@redhat.com>]

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