http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51538
--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-12-14
01:53:00 UTC ---
(In reply to comment #0)
> Here source exaple:
> //////////////////////////////////////////////////
>
> #include <set>
> #include <cstdlib>
> #include <cstdio>
> #include <cstdlib>
You're missing <string.h> and "using namespace std;"
> typedef char FOO[8];
>
> void randomFOO(FOO * uidl)
> {
> for(int i = 0; i < sizeof(uidl); ++i)
Why sizeof(uidl) not sizeof(*uidl)?
> {
> unsigned char byte = rand() * 256 / RAND_MAX;
When RAND_MAX == INT_MAX this calculation will either overflow (causing
undefined behaviour) or will produce zero. That's probably not what you want.
maybe you want:
unsigned char byte = rand() * 256. / RAND_MAX;
> *((char*)uidl + i) = byte;
> }
> }
>
> struct FOOwrapper
> {
> FOOwrapper(const FOO * uidl)
> {
> memcpy(&m_uidl, uidl, sizeof(FOO));
> }
> FOOwrapper(const FOOwrapper & othr)
> {
> memcpy(&m_uidl, &othr.m_uidl, sizeof(FOO));
> }
This copy constructor is unnecessary, the implicit one would do the same thing.
> const FOOwrapper & operator=(const FOOwrapper & othr)
> {
> memcpy(&m_uidl, &othr.m_uidl, sizeof(FOO));
> return *this;
> }
This assignment operator is unnecessary, the implicit one would do the same
thing.
> bool operator<(const FOOwrapper & othr) const
> {
> return memcmp(&m_uidl, &othr.m_uidl, sizeof(FOO));
This is not anti-symmetric so is not suitable for comparing elements of a
std::set, try this:
FOO f1 = { 1 };
FOO f2 = { 2 };
FOOwrapper w1(&f1);
FOOwrapper w2(&f2);
if (w1 < w2) assert( !(w2 < w1) );
Your operator says that x is less than y and also y is less than x.
You probably want:
return memcmp(&m_uidl, &othr.m_uidl, sizeof(FOO)) < 0;
> }
>
> FOO m_uidl;
> };
>
> std::set<FOOwrapper> fooset;
>
> static const int size = 5;
>
> //#define SIMPLE_INSERT
>
> int main(int argc, char *argv[])
> {
> FOOwrapper * wrap = static_cast<FOOwrapper
> *>(malloc(size*sizeof(FOOwrapper)));
> for(int i = 0; i < size; ++i)
> {
> FOO uidl;
> randomFOO(&uidl);
> new (wrap + i) FOOwrapper(&uidl);
> #ifdef SIMPLE_INSERT
> fooset.insert(wrap[i]);
> #endif
> }
> #ifndef SIMPLE_INSERT
> fooset.insert(wrap, wrap + size);
> #endif
> printf("size: %d\n\n", fooset.size());
>
> std::set<FOOwrapper>::const_iterator it = fooset.begin();
> for(int i = 0; it != fooset.end(); ++it,++i)
> printf("%d\n", i);
> free(wrap);
> return 0;
> }
>
> //////////////////////////////////////////////////
>
> if SIMPLE_INSERT is not defined then output will be:
>
> size: <value of variable size>
>
> 0
> 1
>
> No matter how many elements being inserted output always is the same. In other
> words we are limited to iterating only trough 2 elements.
>
> if SIMPLE_INSERT is defined, ie used std::set::insert(const value_type& x)
> method, then iterating through collection is proper.
I get the same result even when SIMPLE_INSERT is defined, because all FOO
objects consist entirely of zeros, so you try to insert duplicates.
> This source code, compiled with VSC++ 2010 works fine in both cases.
The source code is totally broken. With the changes above to randomFOO and
operator< it works.
