public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/47305] New: std::vector::erase() destroys the wrong element!
@ 2011-01-15 2:21 shockema at gmail dot com
2011-01-15 4:33 ` [Bug libstdc++/47305] " shockema at gmail dot com
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: shockema at gmail dot com @ 2011-01-15 2:21 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47305
Summary: std::vector::erase() destroys the wrong element!
Product: gcc
Version: 4.2.1
Status: UNCONFIRMED
Severity: major
Priority: P3
Component: libstdc++
AssignedTo: unassigned@gcc.gnu.org
ReportedBy: shockema@gmail.com
In the C++ stdlib distribution included with Mac OS X (Darwin 10.5.0 i386), the
implementation of std::vector::erase() from vector.tcc lines 106-116 is shown
here:
template<typename _Tp, typename _Alloc>
typename vector<_Tp, _Alloc>::iterator
vector<_Tp, _Alloc>::
erase(iterator __position)
{
if (__position + 1 != end())
std::copy(__position + 1, end(), __position);
--this->_M_impl._M_finish;
this->_M_impl.destroy(this->_M_impl._M_finish);
return __position;
}
Note that "destroy()" will be called for the element that is *last* in the
vector prior to the call to this erase(), instead of being called for the
element pointed to by __position. I believe this is incorrect -- I think it
should instead call destroy() for the element pointed to by __position. For
simple POD types, this isn't that big of a deal, but for classes where the
destructors have side effects (such as smart pointers), it can be critical.
The following code illustrates the problem:
#include <vector>
#include <iostream>
class MyClass
{
int m_x;
public:
MyClass(int x) : m_x(x) { }
~MyClass()
{
std::cerr << "Destroying with m_x=" << m_x << std::endl;
}
};
int main(void)
{
std::vector<MyClass> testvect;
testvect.reserve(8);
testvect.push_back(MyClass(1));
testvect.push_back(MyClass(2));
testvect.push_back(MyClass(3));
testvect.push_back(MyClass(4));
testvect.push_back(MyClass(5));
std::cerr << "ABOUT TO DELETE #3:" << std::endl;
testvect.erase(testvect.begin() + 2);
std::cerr << "DONE WITH DELETE." << std::endl;
return 0;
}
When I compile this with g++ version 4.2.1 (no command line arguments) on my
Mac, it produces the following when I run it:
Destroying with m_x=1
Destroying with m_x=2
Destroying with m_x=3
Destroying with m_x=4
Destroying with m_x=5
ABOUT TO DELETE #3:
Destroying with m_x=5
DONE WITH DELETE.
Destroying with m_x=1
Destroying with m_x=2
Destroying with m_x=4
Destroying with m_x=5
Note that the key line after the "ABOUT TO DELETE #3" message shows that the
destructor was actually called for the fifth thing I added. Importantly, the
destructor for #3 is never called!!
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Bug libstdc++/47305] std::vector::erase() destroys the wrong element!
2011-01-15 2:21 [Bug libstdc++/47305] New: std::vector::erase() destroys the wrong element! shockema at gmail dot com
@ 2011-01-15 4:33 ` shockema at gmail dot com
2011-01-15 6:33 ` pinskia at gcc dot gnu.org
2011-01-15 12:43 ` redi at gcc dot gnu.org
2 siblings, 0 replies; 4+ messages in thread
From: shockema at gmail dot com @ 2011-01-15 4:33 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47305
--- Comment #1 from shockema at gmail dot com 2011-01-15 02:21:23 UTC ---
It appears that the version of erase that takes a range (two iterators) also
has a similar problem.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Bug libstdc++/47305] std::vector::erase() destroys the wrong element!
2011-01-15 2:21 [Bug libstdc++/47305] New: std::vector::erase() destroys the wrong element! shockema at gmail dot com
2011-01-15 4:33 ` [Bug libstdc++/47305] " shockema at gmail dot com
@ 2011-01-15 6:33 ` pinskia at gcc dot gnu.org
2011-01-15 12:43 ` redi at gcc dot gnu.org
2 siblings, 0 replies; 4+ messages in thread
From: pinskia at gcc dot gnu.org @ 2011-01-15 6:33 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47305
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Known to fail| |4.1.3, 4.3.2
Severity|major |normal
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Bug libstdc++/47305] std::vector::erase() destroys the wrong element!
2011-01-15 2:21 [Bug libstdc++/47305] New: std::vector::erase() destroys the wrong element! shockema at gmail dot com
2011-01-15 4:33 ` [Bug libstdc++/47305] " shockema at gmail dot com
2011-01-15 6:33 ` pinskia at gcc dot gnu.org
@ 2011-01-15 12:43 ` redi at gcc dot gnu.org
2 siblings, 0 replies; 4+ messages in thread
From: redi at gcc dot gnu.org @ 2011-01-15 12:43 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47305
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution| |INVALID
--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-15 11:08:06 UTC ---
(In reply to comment #0)
> In the C++ stdlib distribution included with Mac OS X (Darwin 10.5.0 i386), the
GCC 4.2 is no longer maintained, you should either try a current release or
report bugs to Apple.
> implementation of std::vector::erase() from vector.tcc lines 106-116 is shown
> here:
>
> template<typename _Tp, typename _Alloc>
> typename vector<_Tp, _Alloc>::iterator
> vector<_Tp, _Alloc>::
> erase(iterator __position)
> {
> if (__position + 1 != end())
> std::copy(__position + 1, end(), __position);
> --this->_M_impl._M_finish;
> this->_M_impl.destroy(this->_M_impl._M_finish);
> return __position;
> }
>
>
> Note that "destroy()" will be called for the element that is *last* in the
> vector prior to the call to this erase(), instead of being called for the
> element pointed to by __position. I believe this is incorrect -- I think it
> should instead call destroy() for the element pointed to by __position.
No, the element at position is overwritten by the call to std::copy()
> For
> simple POD types, this isn't that big of a deal, but for classes where the
> destructors have side effects (such as smart pointers), it can be critical.
>
> The following code illustrates the problem:
>
>
> #include <vector>
> #include <iostream>
>
> class MyClass
> {
> int m_x;
> public:
> MyClass(int x) : m_x(x) { }
> ~MyClass()
> {
> std::cerr << "Destroying with m_x=" << m_x << std::endl;
> }
> };
>
> int main(void)
> {
> std::vector<MyClass> testvect;
> testvect.reserve(8);
> testvect.push_back(MyClass(1));
> testvect.push_back(MyClass(2));
> testvect.push_back(MyClass(3));
> testvect.push_back(MyClass(4));
> testvect.push_back(MyClass(5));
>
> std::cerr << "ABOUT TO DELETE #3:" << std::endl;
>
> testvect.erase(testvect.begin() + 2);
>
> std::cerr << "DONE WITH DELETE." << std::endl;
>
> return 0;
> }
>
>
> When I compile this with g++ version 4.2.1 (no command line arguments) on my
> Mac, it produces the following when I run it:
>
> Destroying with m_x=1
> Destroying with m_x=2
> Destroying with m_x=3
> Destroying with m_x=4
> Destroying with m_x=5
> ABOUT TO DELETE #3:
> Destroying with m_x=5
> DONE WITH DELETE.
> Destroying with m_x=1
> Destroying with m_x=2
> Destroying with m_x=4
> Destroying with m_x=5
As you can see, the results are correct, the vector contains {1,2,4,5}
> Note that the key line after the "ABOUT TO DELETE #3" message shows that the
> destructor was actually called for the fifth thing I added. Importantly, the
> destructor for #3 is never called!!
Doesn't matter, the requirements of std::vector do not say that must happen
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-01-15 11:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-15 2:21 [Bug libstdc++/47305] New: std::vector::erase() destroys the wrong element! shockema at gmail dot com
2011-01-15 4:33 ` [Bug libstdc++/47305] " shockema at gmail dot com
2011-01-15 6:33 ` pinskia at gcc dot gnu.org
2011-01-15 12:43 ` redi at gcc dot gnu.org
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).