* sized delete in _Temporary_buffer<>
@ 2019-07-18 5:41 François Dumont
2019-07-18 10:18 ` Jonathan Wakely
2019-07-19 20:16 ` Morwenn Ed
0 siblings, 2 replies; 4+ messages in thread
From: François Dumont @ 2019-07-18 5:41 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 437 bytes --]
As we adopted the sized deallocation in the new_allocator why not doing
the same in _Temporary_buffer<>.
   * include/bits/stl_tempbuf.h (__detail::__return_temporary_buffer):
New.
   (~_Temporary_buffer()): Use latter.
   (_Temporary_buffer(_FIterator, size_type)): Likewise.
Tested w/o activating sized deallocation. I'll try to run tests with
this option activated.
Ok to commit ?
François
[-- Attachment #2: temp_buf.patch --]
[-- Type: text/x-patch, Size: 2592 bytes --]
diff --git a/libstdc++-v3/include/bits/stl_tempbuf.h b/libstdc++-v3/include/bits/stl_tempbuf.h
index b6ad9ee6a46..bb7c2cd1334 100644
--- a/libstdc++-v3/include/bits/stl_tempbuf.h
+++ b/libstdc++-v3/include/bits/stl_tempbuf.h
@@ -63,6 +63,21 @@ namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
+ namespace __detail
+ {
+ template<typename _Tp>
+ inline void
+ __return_temporary_buffer(_Tp* __p,
+ size_t __len __attribute__((__unused__)))
+ {
+#if __cpp_sized_deallocation
+ ::operator delete(__p, __len);
+#else
+ ::operator delete(__p);
+#endif
+ }
+ }
+
/**
* @brief Allocates a temporary buffer.
* @param __len The number of objects of type Tp.
@@ -112,7 +127,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return_temporary_buffer(_Tp* __p)
{ ::operator delete(__p); }
-
/**
* This class is used in two places: stl_algo.h and ext/memory,
* where it is wrapped as the temporary_buffer class. See
@@ -165,7 +179,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
~_Temporary_buffer()
{
std::_Destroy(_M_buffer, _M_buffer + _M_len);
- std::return_temporary_buffer(_M_buffer);
+ std::__detail::__return_temporary_buffer(_M_buffer, _M_len);
}
private:
@@ -185,7 +199,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__ucr(_Pointer __first, _Pointer __last,
_ForwardIterator __seed)
{
- if(__first == __last)
+ if (__first == __last)
return;
_Pointer __cur = __first;
@@ -244,22 +258,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Temporary_buffer(_ForwardIterator __seed, size_type __original_len)
: _M_original_len(__original_len), _M_len(0), _M_buffer(0)
{
- __try
- {
- std::pair<pointer, size_type> __p(std::get_temporary_buffer<
- value_type>(_M_original_len));
- _M_buffer = __p.first;
- _M_len = __p.second;
- if (_M_buffer)
- std::__uninitialized_construct_buf(_M_buffer, _M_buffer + _M_len,
- __seed);
- }
- __catch(...)
+ std::pair<pointer, size_type> __p(
+ std::get_temporary_buffer<value_type>(_M_original_len));
+
+ if (__p.first)
{
- std::return_temporary_buffer(_M_buffer);
- _M_buffer = 0;
- _M_len = 0;
- __throw_exception_again;
+ __try
+ {
+ std::__uninitialized_construct_buf(__p.first, __p.first + __p.second,
+ __seed);
+ _M_buffer = __p.first;
+ _M_len = __p.second;
+ }
+ __catch(...)
+ {
+ std::__detail::__return_temporary_buffer(__p.first, __p.second);
+ __throw_exception_again;
+ }
}
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: sized delete in _Temporary_buffer<>
2019-07-18 5:41 sized delete in _Temporary_buffer<> François Dumont
@ 2019-07-18 10:18 ` Jonathan Wakely
2019-07-19 20:16 ` Morwenn Ed
1 sibling, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2019-07-18 10:18 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 18/07/19 07:41 +0200, François Dumont wrote:
>As we adopted the sized deallocation in the new_allocator why not
>doing the same in _Temporary_buffer<>.
>
>Â Â Â * include/bits/stl_tempbuf.h
>(__detail::__return_temporary_buffer): New.
>Â Â Â (~_Temporary_buffer()): Use latter.
>Â Â Â (_Temporary_buffer(_FIterator, size_type)): Likewise.
>
>Tested w/o activating sized deallocation. I'll try to run tests with
>this option activated.
As the manual says, it's enabled by default for C++14 and later.
>Ok to commit ?
OK for trunk, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: sized delete in _Temporary_buffer<>
2019-07-18 5:41 sized delete in _Temporary_buffer<> François Dumont
2019-07-18 10:18 ` Jonathan Wakely
@ 2019-07-19 20:16 ` Morwenn Ed
2019-07-19 21:38 ` François Dumont
1 sibling, 1 reply; 4+ messages in thread
From: Morwenn Ed @ 2019-07-19 20:16 UTC (permalink / raw)
To: François Dumont, libstdc++, gcc-patches
If I'm not mistaken this patch allocates N*sizeof(_Tp) bytes of storage and deallocates N bytes when sized deallocation is enabled?
Shouldn't __return_temporary_buffer deallocate N*sizeof(_Tp) instead to match the value passed to new?
________________________________
De : libstdc++-owner@gcc.gnu.org <libstdc++-owner@gcc.gnu.org> de la part de François Dumont <frs.dumont@gmail.com>
Envoyé : jeudi 18 juillet 2019 07:41
À : libstdc++@gcc.gnu.org <libstdc++@gcc.gnu.org>; gcc-patches <gcc-patches@gcc.gnu.org>
Objet : sized delete in _Temporary_buffer<>
As we adopted the sized deallocation in the new_allocator why not doing
the same in _Temporary_buffer<>.
* include/bits/stl_tempbuf.h (__detail::__return_temporary_buffer):
New.
(~_Temporary_buffer()): Use latter.
(_Temporary_buffer(_FIterator, size_type)): Likewise.
Tested w/o activating sized deallocation. I'll try to run tests with
this option activated.
Ok to commit ?
François
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: sized delete in _Temporary_buffer<>
2019-07-19 20:16 ` Morwenn Ed
@ 2019-07-19 21:38 ` François Dumont
0 siblings, 0 replies; 4+ messages in thread
From: François Dumont @ 2019-07-19 21:38 UTC (permalink / raw)
To: Morwenn Ed, libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1396 bytes --]
(2nd sent attempt as text this time.)
Good spot, fixed with attached patch, committed as trivial.
2019-07-19 François Dumont <fdumont@gcc.gnu.org>
   * include/bits/stl_tempbuf.h (__detail::__return_temporary_buffer): Fix
   sized deallocation size computation.
On 7/19/19 9:46 PM, Morwenn Ed wrote:
> If I'm not mistaken this patch allocates N*sizeof(_Tp) bytes of
> storage and deallocates N bytes when sized deallocation is enabled?
>
> Shouldn't __return_temporary_buffer deallocate N*sizeof(_Tp) instead
> to match the value passed to new?
>
> ------------------------------------------------------------------------
> *De :* libstdc++-owner@gcc.gnu.org <libstdc++-owner@gcc.gnu.org> de la
> part de François Dumont <frs.dumont@gmail.com>
> *Envoyé :* jeudi 18 juillet 2019 07:41
> *À :* libstdc++@gcc.gnu.org <libstdc++@gcc.gnu.org>; gcc-patches
> <gcc-patches@gcc.gnu.org>
> *Objet :* sized delete in _Temporary_buffer<>
> As we adopted the sized deallocation in the new_allocator why not doing
> the same in _Temporary_buffer<>.
>
> Â Â Â Â * include/bits/stl_tempbuf.h (__detail::__return_temporary_buffer):
> New.
> Â Â Â Â (~_Temporary_buffer()): Use latter.
> Â Â Â Â (_Temporary_buffer(_FIterator, size_type)): Likewise.
>
> Tested w/o activating sized deallocation. I'll try to run tests with
> this option activated.
>
> Ok to commit ?
>
> François
>
[-- Attachment #2: temp_buf.patch --]
[-- Type: text/x-patch, Size: 483 bytes --]
diff --git a/libstdc++-v3/include/bits/stl_tempbuf.h b/libstdc++-v3/include/bits/stl_tempbuf.h
index bb7c2cd1334..ce3f3624437 100644
--- a/libstdc++-v3/include/bits/stl_tempbuf.h
+++ b/libstdc++-v3/include/bits/stl_tempbuf.h
@@ -71,7 +71,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
size_t __len __attribute__((__unused__)))
{
#if __cpp_sized_deallocation
- ::operator delete(__p, __len);
+ ::operator delete(__p, __len * sizeof(_Tp));
#else
::operator delete(__p);
#endif
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-19 21:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-18 5:41 sized delete in _Temporary_buffer<> François Dumont
2019-07-18 10:18 ` Jonathan Wakely
2019-07-19 20:16 ` Morwenn Ed
2019-07-19 21:38 ` François Dumont
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).