porting from vc6 to vc2005

0a8e708710b4c589dce4b774aa3e1271
0
kyuzo 101 Mar 18, 2006 at 21:03

hi all, i’m new here.

here’s my problem:
i have a GUI library based on windows; all the widgets inherits from a base class; this class has a list of child widgets (a std::vector, actualy), and its destructor also deletes all the widgets children, like this:

if(parent){//remove this kid from the parent’s list
std::vector<WinObject *>::iterator it;
for(it=parent->children.begin();it!=parent->children.end();it++)
if((*it)==this){
parent->children.erase(it);
break;
}
}
std::vector<WinObject *>::iterator it=children.begin();
while(children.size()){
delete(*it);
}

While this works perfectly in vc6 and in mingw (gcc 3.4.2) it fails in vc2005 express edition, with a “vector iterator not dereferencable” error. Now, i have no idea, it is vc2005, it is my code?
Thank you for any help.

24 Replies

Please log in or register to post a reply.

80a58d4b8c82de7ee25c067854fea766
0
mysticman 101 Mar 18, 2006 at 23:04

hi Kyuzo! (also I am Italian)

The problem I don’t know whether to resolve it, but I have had to abdicate to a project c++ with the vc2005pro and to bring it on vc2003. Too many problems, especially if it has some functions that ask for the directx SDK….
sorry for my bad engrish! :blush:

0a8e708710b4c589dce4b774aa3e1271
0
kyuzo 101 Mar 18, 2006 at 23:50

most probably is my code, not vc2005; i’ll come back with the solution, if i’ll find it…hopefuly.

Da26e799270ce5e8b62659ed77b11cef
0
Axel 101 Mar 18, 2006 at 23:57

Is that a compile- or runtime error?

std::vector<WinObject *>::iterator it=children.begin();
while(children.size()) delete(*it);

I don’t even understand why this doesn’t generate an infinite loop. The size of the children vector will not decrease if you delete the objects that it points to.

Edit: You don’t even increment the iterator in the loop. WTF?

0a8e708710b4c589dce4b774aa3e1271
0
kyuzo 101 Mar 19, 2006 at 00:08

that’s a runtime error.
it doesnt generate an infinite loop, as there’s a parent->children.erase(it); before the code you’ve quoted.

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 168 Mar 19, 2006 at 04:23

No, Axel is right. The original code, indented for more clarity, is

if(parent){//remove this kid from the parent's list
    std::vector<WinObject *>::iterator it;
    for(it=parent->children.begin();it!=parent->children.end();it++)
        if((*it)==this){
            parent->children.erase(it);
            break;
        }
}
std::vector<WinObject *>::iterator it=children.begin();
while(children.size()){
    delete(*it);
}

First of all, the loop that tries to remove ‘this’ from ‘parent’ isn’t guaranteed to work properly because the iterator is invalidated by the call to erase(). And as Axel says, the second loop absolutely doesn’t do what it is supposed to. What you probably want is like this:

if (parent)
{
    iterator i = parent->children.begin();
    while (i != parent->children.end())
        if (*i == this) i = parent->children.erase(i); else ++i;
}
iterator i = children.begin(), end = children.end();
for (; i != end; ++i)
    delete *i;

Note that erase() invalidates the iterator passed into it, and in a vector it also invalidates all the iterators after it, including end() (and potentially all iterators period, if a reallocation to smaller size occurs). With a list, this wouldn’t be a problem. In either case, it returns a valid iterator to the position after the deleted item.

0a8e708710b4c589dce4b774aa3e1271
0
kyuzo 101 Mar 19, 2006 at 13:11

i thought i was decent in c++, but i was wrong. cant make it work.
here a small program that illustrates my problem:

#include<list>


class Base{
    std::list<Base *> children;
    Base *parent;
public:
    Base(Base *p=0);
    ~Base();
};

Base::Base(Base *p):parent(p){
    if(p)
        p->children.push_back(this);
}

Base::~Base(){
    std::list<Base *>::iterator i,end;
    if (parent){
    i = parent->children.begin();
    while (i != parent->children.end())
        if (*i == this) i = parent->children.erase(i); else ++i;
    }
    i = children.begin(), end = children.end();
    for (; i != end; ++i)
    delete *i;
}

int main(){
    Base *window, *menu;
    window=new Base();
    menu=new Base(window);
    delete window;
}

the destructor should clean all his children, it does not.
i’d appreciate any help you can give (again)

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 168 Mar 19, 2006 at 17:56

That destructor code definitely should call the destructor on each child and reclaims the memory allocated by the child.

It does NOT clear the vector, so at the end of the destructor it will still contain a list of (no longer valid) pointers to children. But the vector’s destructor will take care of this, you don’t have to do it yourself.

Also, if you are inheriting anything from Base, you need to make its destructor virtual. Otherwise the derived class’s destructor will not be called.

0a8e708710b4c589dce4b774aa3e1271
0
kyuzo 101 Mar 19, 2006 at 18:03

do you mean that this code is working for you!? it gaves me a “list iterator not incrementable” error at runtime.

0a8e708710b4c589dce4b774aa3e1271
0
kyuzo 101 Mar 19, 2006 at 18:15

just to make things clear, suppose the program is like:

int main(){
    Base *window, *menu,*menitem;
    window=new Base();
    menu=new Base(window);
    menuitem=new Base(menu);
        delete menuitem;
    delete window;
}

for the earlier version, a destructor like

Base::~Base(){
  for(i=children.begin();i!=children.end();++i)
    delete *i;
}

would do.

thanks.

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 168 Mar 19, 2006 at 18:27

Okay, I’ve looked at it more closely and I see what’s happening. The call to ‘delete *i’ causes the child’s destructor to run, and the child removes itself from the parent’s list. This invalidates the iterator in the parent’s destructor, and when control returns there the system chokes.

Is there any situation in which you would want to remove a control from its parent without deleting the parent? If not, I suggest just using the destructor you posted above. It will recursively delete all children upon deletion of the top-level control. So there would be no need to call ‘delete menuitem’, as ‘delete window’ will delete all the controls in the correct order.

Otherwise, I suggest creating a new member function whose purpose is to remove a child from a control. You could tell this member not to do anything if it was called while destructing, e.g. by using a private flag.

Another approach might be to set the child pointer in the list to NULL when the child is deleted. But then whenever you looped over the children you would have to check to be sure you skipped over null pointers.

0a8e708710b4c589dce4b774aa3e1271
0
kyuzo 101 Mar 19, 2006 at 18:35

@Reedbeta

Is there any situation in which you would want to remove a control from its parent without deleting the parent?

The idea was to make this library work like a GUI builder too, like when i #define DESIGN, i can drag&drop, modify, delete the widgets. It kind of worked, then i thought to switch to vc++2005 and it stopped working. Anyway, thank you all for your help, i will try again.

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 168 Mar 19, 2006 at 20:32

Well, I hope you get it working. Just to let you know, I think the code was incorrect from the beginning; it was just pure chance that it happened to work right under VC6.

0a8e708710b4c589dce4b774aa3e1271
0
kyuzo 101 Mar 19, 2006 at 22:57

ok, here’s what i have:

std::vector<WinObject *>::iterator it;
if(parent){
    it=parent->children.begin();
    while(it!=parent->children.end())
        if(this==*it){
            parent->children.erase(it);
            break;
        }
}
while(children.size()){
    it=children.begin();
    delete *it;
}

@Reedbeta:
i know you dont like it (though i cant see any obvious error), but all the children widgets get deleted, it works in both vc6 and vc2005, so i guess i leave it as it is right now, at least for the moment.

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 168 Mar 20, 2006 at 01:19

kyuzo,

That may work when you are deleting the parent, so it deletes all its children in order, but it won’t work if you want to delete a single arbitrary child (like in your GUI-builder idea). You’re not incrementing the iterator in the first loop, so it will only find ‘this’ in parent->children if ‘this’ is the first element!

0a8e708710b4c589dce4b774aa3e1271
0
kyuzo 101 Mar 20, 2006 at 12:00

oh, yes, thanks for noticing it! it needs an “else ++it;”
anyway, it looks that puting the “it=children.begin();” inside the while loop got the job done. dont ask me why.

340bf64ac6abda6e40f7e860279823cb
0
_oisyn 101 Mar 20, 2006 at 13:38

Also, you should better use empty() to test whether a container is empty or not, as size() is not guaranteed to be a constant time function (while empty() is)

77078a51228688ee3cc87e539c4ff235
0
Faelenor 101 Mar 20, 2006 at 16:26

VC 2005 is a lot more standard than the previous versions. If something doesn’t work in 2005 and was working in 2003, then you know that you were doing something wrong. The STL implementation in 2003 was really bad.

kyuzo: maybe a book about stl would be useful… I suggest Effective STL.

E54629ba7ececd78c241651d26cb8903
0
CobraLionz 101 Mar 20, 2006 at 16:33

This code does not compute.
@kyuzo

ok, here’s what i have:

std::vector<WinObject *>::iterator it;
if(parent){
    it=parent->children.begin();
    while(it!=parent->children.end())
        if(this==*it){
            parent->children.erase(it);
            break;
        }
}
while(children.size()){
    it=children.begin();
    delete *it;
}

@Reedbeta:
i know you dont like it (though i cant see any obvious error), but all the children widgets get deleted, it works in both vc6 and vc2005, so i guess i leave it as it is right now, at least for the moment.

0a8e708710b4c589dce4b774aa3e1271
0
kyuzo 101 Mar 20, 2006 at 16:44

uh, what do you mean? it works for me (add an ++it in the 1st loop, of course).

340bf64ac6abda6e40f7e860279823cb
0
_oisyn 101 Mar 20, 2006 at 17:03

@Faelenor

VC 2005 is a lot more standard than the previous versions. If something doesn’t work in 2005 and was working in 2003, then you know that you were doing something wrong. The STL implementation in 2003 was really bad.

Actually the STL implementation of 2003 was quite good. The one from 2002 was bad and VC6 and earlier even worse, mainly because they lack partial template specialization.

A8433b04cb41dd57113740b779f61acb
0
Reedbeta 168 Mar 20, 2006 at 19:30

Cobra,

It actually does work with a slight modification to the first loop. (kyuzo, you need not only to add else ++i, but also make i = parent->children.erase). It’s because in the second loop, the destructor of the child removes that child from the parent’s list, and so the call to ‘delete *i’ actually shortens the list. The code is still not very clear, and kyuzo, you shouldn’t be using any code if you don’t understand exactly what it does.

77078a51228688ee3cc87e539c4ff235
0
Faelenor 101 Mar 20, 2006 at 20:00

@.oisyn

Actually the STL implementation of 2003 was quite good. The one from 2002 was bad and VC6 and earlier even worse, mainly because they lack partial template specialization.

Well, at work, we had a lot of problems when we moved to 2005. A lot of our code was not always using the STL as it should according to the standard, but was compiling and running. Those were subtle errors in our STL use, but they were real errors that shouldn’t compile nor run. But I can’t remember exactly what were the errors…

Actually, you are right. The STL implementation of 2003 was not so bad. The main difference with 2005 is that it does more error checking now, both at compile time and runtime.

Da26e799270ce5e8b62659ed77b11cef
0
Axel 101 Mar 20, 2006 at 21:37

VC 2005 has error checking in the STL (Which is a good thing. Pointed me to 2 or 3 bugs which l overlooked as well in one of my larger projects) VC 2003 does not. Thats the major difference I noticed.

Edit: As Faelenor already said :)

6d318bb67270aa12b325e2cd7b64ff7a
0
pater 101 Mar 23, 2006 at 21:17

@Axel

VC 2005 has error checking in the STL (Which is a good thing. Pointed me to 2 or 3 bugs which l overlooked as well in one of my larger projects) VC 2003 does not. Thats the major difference I noticed.

Well, sometimes, this error checking is also nasty and prevents (proper) code from compilling under 2005. I don’t remember the exact situation, but there was a constructor of a class, which took a single string argument in 2003. This allowed this constructor to be called implicitly. Now in 2005 (debug mode) the constructor takes two arguments, one of which is for error checking. This prevents the constructor to be called, and the compiler complains that there was no conversion from string to the target type available. The solution was to add some preprocessor statement which disabled the extra debug parameters, whose meaning I didn’t even understand.