On Tue, Jul 23, 2019 at 1:30 AM Boyarinov, Konstantin via Std-Proposals <std-proposals@lists.isocpp.org> 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. 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