commit 2dd24000b647d315b560320a680a8d8814e731f4
author bnc <bnc@google.com> Mon Apr 05 11:13:54 2021 -0700
committer Copybara-Service <copybara-worker@google.com> Mon Apr 05 11:14:57 2021 -0700
tree dbda8b9a2552ceabbb59d6daae97339a41a67391
parent e7534348b1ff729a562f2c23e191c5b35d4d3e5e

Improve HpackEntry class constructors.

Make constructor take std::string.  This forces the call sites to explicitly
copy, making it clear that this class does not store reference (unlike
HpackLookupEntry).

Make it explicitly movable.  This is trivial since the absl::string_view member
pointing to the std::string member has been removed at cr/363705447.

Make it non-copyable.  This ensures that it is always moved.

Remove default constructor.  It is never used, and it turns out that it is not
necessary for STL containers.

Making the type movable allows the use push() instead of emplace() cheaply in
accordance with guidance that Dianna brought to my attention at cl/363455100.

I was contemplating changing this class into a struct and removing the name()
and value() accessors, like HpackLookupEntry.  However, that introduces a subtle
issue that I have already ran into in the past when I was exploring changing the
accessor return types from absl::string_view to const std::string&.  When
inserting into static_name_index_ and dynamic_name_index_ maps, std::make_pair
is used as customary for map insertions:

http://google3/third_party/spdy/core/hpack/hpack_static_table.cc?l=41&rcl=364626329
http://google3/third_party/spdy/core/hpack/hpack_header_table.cc?l=169&rcl=364636382

However, if std::string name member is passed into make_pair, or the const
std::string& return value of an accessor, then make_pair makes a temporary
<std::string, size_t> pair by copying the string.  This gets implicitly
converted into an <absl::string_view, size_t> pair by insert(), where the
string_view points to the temporary string which is destroyed right after the
string_view is inserted into the map, leaving a dangling reference.  I could do
an explicit string_view conversion to avoid this, but instead I decided to keep
the less error-prone API.
PiperOrigin-RevId: 366834546
Change-Id: I00636f034010e74d8db0ab9a5ee3077148988349