http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56112
--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-01-27 16:25:36 UTC --- (In reply to comment #5) > Uhhmm, I wasn't considering the fact that the testcases added for 53339 are > now > xfailed?!? That was me: http://gcc.gnu.org/ml/libstdc++/2013-01/msg00035.html > So what? If that's the case (for now at least) we could as well > revert the core of the 53339 changes, that is go back to the 4.7 situation and > just use std::_Select1st<value_type>?!? Yes, I suppose we could, but we'd need to revert the changes to stl_function.h so that it doesn't derive from unary_function in C++11 mode. I think having a simple __detail::_Select1st<_Pair> that just does what the hash tables need is cleaner, and it can be made simpler than your patch because it doesn't need to use perfect forwarding: template<typename _Pair> struct _Select1st { auto operator()(const _Pair& __x) const -> decltype(std::get<0>(__x)) { return std::get<0>(__x); } }; Or we could simplify it even further const typename _Pair::first_type& operator()(const _Pair& __x) const { return __x.first); } This would prevent fancy custom uses of _Hashtable, for example _Hashtable<int, std::tuple<int, int, int>, ...>, but I don't think we need or want to do that anyway. My testcase for this bug reveals another problem in an area we've had recurring problems: _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, _H1, _H2, _Hash, _RehashPolicy, _Traits>:: _M_insert(_Arg&& __v, std::true_type) { const key_type& __k = this->_M_extract()(__v); __hash_code __code = this->_M_hash_code(__k); size_type __bkt = _M_bucket_index(__k, __code); __node_type* __n = _M_find_node(__bkt, __k, __code); With my testcase __k is a dangling reference, because the call to _M_extract creates a temporary, __k binds to a member of that temporary, then the temporary goes out of scope. Valgrind will show errors for this modified testcase which allocates memory in the temporary: #include <unordered_map> #include <string> struct S { operator std::pair<const std::string, int>() const { return {"a", 0}; } }; int main() { S s[1]; std::unordered_map<std::string, int> m(s, s+1); // error } To fix this we need to extract the key and set __code, __bkt and __n in a single statement before the temporary goes out of scope, e.g. using a lambda just for a proof of concept: __hash_code __code; size_type __bkt; __node_type* __n; auto __find_node = [&](const key_type& __k) { __code = this->_M_hash_code(__k); __bkt = _M_bucket_index(__k, __code); __n = _M_find_node(__bkt, __k, __code); }; __find_node(this->_M_extract()(value_type(__v))); So I think we want something like your comment 4 attachment, with a simpler _Select1st, and fixing the memory bug. I'm testing that now ...
