C++ Logo

std-proposals

Advanced search

Re: Reasons for adding insert_return_type in associative containers

From: Arthur O'Dwyer <arthur.j.odwyer_at_[hidden]>
Date: Tue, 23 Jul 2019 02:22:17 -0400
On Tue, Jul 23, 2019 at 1:30 AM Boyarinov, Konstantin via Std-Proposals <
std-proposals_at_[hidden]> wrote:

> Hello,
>
> I have a small question regarding *insert_return_type *member type in STL
> ordered and unordered associative containers.
>
> Node handling mechanism introduced two new overloads for * insert*:
>
>
>
> *insert_return_type insert(node_type&&
> nh); (1)*
>
> *iterator insert(const_iterator hint, node_type&& nh);
> (2)*
>
>
[where the `node` member of the returned `insert_return_type` gets the old
value of `nh` iff the insert does not happen]


> Overload (2) inserts a node owned by *nh *into the container and if the
> insertion fails *nh *retains ownership on the node.
>
>
>
> So the question is why such inconsistence is needed?
>

Well, overload (1) is clearly a better interface than overload (2).
 "Conditionally moving-from" a reference parameter is terrible API design.
Why? Because the static analyzer would like to look at a local piece of
code such as

    Widget w;
    foo(std::move(w));
    bar(w); // OOPS

and diagnose the line marked "OOPS" as a use-after-move. If
`foo(std::move(w))` is not a bug, and yet does not leave `w` in a
moved-from state either, then the static analyzer is going to produce a lot
of false positives. This is bad. For the sake of static analysis, we need
"move" to mean "move." When our library forces the programmer to write
`std::move` on a function argument, it had better be because that function
is going to unconditionally move-out-of that argument!

So overload (1) has a good reason to unconditionally move-out-of its rvalue
parameter `nh`. But sometimes it doesn't want to take ownership of the node
referred to by `nh`. In that case, it wants to *return* ownership of the
node back to the caller. So it includes a node handle as part of the
*return* type of the function.

So your question boils down to: Why does overload (2) — the "hinted"
version of `insert` — use a known terrible API design which is deliberately
inconsistent with the good API design of overload (1) — especially given
that both overloads were added in the same version of C++?


I don't know for sure, but I suspect that the answer is that nobody cares
about the "hinted" versions of `insert`. Vendors just ignore the `hint`
parameter; programmers don't call these methods in the first place. So they
were probably not looked at very thoroughly during wording review.

LWG discussion on the Jacksonville (2016) wiki indicates that the good API
for (1) was not part of the original proposal, but was
designed-by-committee in LEWG. Presumably LEWG didn't notice or care about
the "hinted" version because nobody uses it. A few people in LWG at
Jacksonville did notice the asymmetry between (1) and (2), but presumably
didn't care to do "design" work that LEWG was already supposed to have
done, so they shipped it with the asymmetry intact.

I am obliged to point out the date on P0083R3: June 24, 2016, mere days
before the Committee Draft of C++17 was released
<https://herbsutter.com/2016/06/30/trip-report-summer-iso-c-standards-meeting-oulu/>.
We
are now (July 2019) in that same period — the Committee Draft of C++20 was
set in stone at Cologne just a few days ago. And this time we have *even
more* features still undergoing design work, and barely squeaking into the
draft ahead of the deadline with insufficient review time. There will be a
lot more mistakes in C++20 than there were in C++17 (which had more than
C++14, and so on). Being able to spot the mistakes two *years* later
doesn't help, when C++ is on a three-year release cycle with major features
going in in the last few *days* of each cycle!

–Arthur

Received on 2019-07-23 01:24:15