On 26/11/18 12:03 +0000, Jonathan Wakely wrote:
On 24/11/18 22:58 +0100, François Dumont wrote:
Tests have shown a regression. I was building the _ReuseOrAllocNode
instance too early, node ownership was transfered but was still
owned by _Hashtable instance.
This new version pass all tests.
This is why it's worth waiting until tests have run before sending a
patch for review.
Ok to commit ?
OK, but please rename _M_replicate to _M_do_assign or _M_assign_impl
or something that makes it clear this is part of the assignment
operation. The name "replicate" isn't clear.
A comment on the declaration of the new function could be helpful too.
Thanks for finding this leak. I think it's worth backporting the fix,
but for gcc-7-branch and gcc-8-branch it should be just:
--- a/libstdc++-v3/include/bits/hashtable.h
+++ b/libstdc++-v3/include/bits/hashtable.h
@@ -1223,6 +1223,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
[&__roan](__node_type* __n)
{ return __roan(std::move_if_noexcept(__n->_M_v())); });
__ht.clear();
+ if (__former_buckets)
+ _M_deallocate_buckets(__former_buckets, __former_bucket_count);
}
__catch(...)
{
(Plus the improvements to the tests, of course).
Because this is a regression, and we want to fix it on the branches,
I've created a bugzilla to track it:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88199
