Jump to content

  • Log In with Google      Sign In   
  • Create Account

Banner advertising on our site currently available from just $5!


1. Learn about the promo. 2. Sign up for GDNet+. 3. Set up your advert!


#ActualParadigm Shifter

Posted 30 June 2013 - 09:27 AM

 

Anyway, @Moonkis: unless I'm misreading/misunderstanding that stack trace, it's actually crashing on ++it. The problem is that erase() (for both vector and set) will return end() if you just erased the last element. Then the loop will try to ++it (before checking it != end()), which is undefined behavior. You need to check if it == end() (and NOT do ++it if it is).

 

Basically, this can be caused if your program follows this execution order:

  1. auto it = objects.begin()
  2. it != objects.end()
  3. execute loop body
  4. it = objects.erase(it); // Let's say you erased the last element and now erase() returns end()
  5. ++it // It will crash here, because it == end()!
  6. it != objects.end()
    1. Go to #3 if true
    2. Break out of the loop if false

This did the trick, here is the altered code:
 

	if(status == QuadStatus::LEAF) {
		for( auto it = objects.begin(); it != objects.end(); ++it) {
			if(!inBounds((*it)->getX(), (*it)->getY()) ) {
				GameObject* o = (*it);
				it = objects.erase(it);
				parent->addObject(o);
				if ( it == objects.end() ) break;
			}
		}
		return;
	}

Problem is, I don't understand why "if ( it == objects.end() ) break;" is needed!

If the objects.erase() returns "end()" the for-loop should catch it before increasing the iterator, why doesn't it?

 

 

That looks wrong to me, if you erase something the iterator is incremented twice. You want to get rid of ++it from the for(;;) line and add else ++it; if the if statement is not entered.

 

EDIT: Like so... (and the check for .end() isn't needed either now)

if(status == QuadStatus::LEAF) {
		for( auto it = objects.begin(); it != objects.end(); /* no-op */) {
			if(!inBounds((*it)->getX(), (*it)->getY()) ) {
				GameObject* o = (*it);
				it = objects.erase(it);
				parent->addObject(o);
                                // This line is no longer necessary, either
				//if ( it == objects.end() ) break;
			}
                        else
                        {
                                ++it;
                        }
		}
		return;
	}

#2Paradigm Shifter

Posted 30 June 2013 - 09:26 AM

 

Anyway, @Moonkis: unless I'm misreading/misunderstanding that stack trace, it's actually crashing on ++it. The problem is that erase() (for both vector and set) will return end() if you just erased the last element. Then the loop will try to ++it (before checking it != end()), which is undefined behavior. You need to check if it == end() (and NOT do ++it if it is).

 

Basically, this can be caused if your program follows this execution order:

  1. auto it = objects.begin()
  2. it != objects.end()
  3. execute loop body
  4. it = objects.erase(it); // Let's say you erased the last element and now erase() returns end()
  5. ++it // It will crash here, because it == end()!
  6. it != objects.end()
    1. Go to #3 if true
    2. Break out of the loop if false

This did the trick, here is the altered code:
 

	if(status == QuadStatus::LEAF) {
		for( auto it = objects.begin(); it != objects.end(); ++it) {
			if(!inBounds((*it)->getX(), (*it)->getY()) ) {
				GameObject* o = (*it);
				it = objects.erase(it);
				parent->addObject(o);
				if ( it == objects.end() ) break;
			}
		}
		return;
	}

Problem is, I don't understand why "if ( it == objects.end() ) break;" is needed!

If the objects.erase() returns "end()" the for-loop should catch it before increasing the iterator, why doesn't it?

 

 

That looks wrong to me, if you erase something the iterator is incremented twice. You want to get rid of ++it from the for(;;) line and add else ++it; if the if statement is not entered.

 

EDIT: Like so...

if(status == QuadStatus::LEAF) {
		for( auto it = objects.begin(); it != objects.end(); /* no-op */) {
			if(!inBounds((*it)->getX(), (*it)->getY()) ) {
				GameObject* o = (*it);
				it = objects.erase(it);
				parent->addObject(o);
				if ( it == objects.end() ) break;
			}
                        else
                        {
                                ++it;
                        }
		}
		return;
	}

#1Paradigm Shifter

Posted 30 June 2013 - 09:21 AM

 

Anyway, @Moonkis: unless I'm misreading/misunderstanding that stack trace, it's actually crashing on ++it. The problem is that erase() (for both vector and set) will return end() if you just erased the last element. Then the loop will try to ++it (before checking it != end()), which is undefined behavior. You need to check if it == end() (and NOT do ++it if it is).

 

Basically, this can be caused if your program follows this execution order:

  1. auto it = objects.begin()
  2. it != objects.end()
  3. execute loop body
  4. it = objects.erase(it); // Let's say you erased the last element and now erase() returns end()
  5. ++it // It will crash here, because it == end()!
  6. it != objects.end()
    1. Go to #3 if true
    2. Break out of the loop if false

This did the trick, here is the altered code:
 

	if(status == QuadStatus::LEAF) {
		for( auto it = objects.begin(); it != objects.end(); ++it) {
			if(!inBounds((*it)->getX(), (*it)->getY()) ) {
				GameObject* o = (*it);
				it = objects.erase(it);
				parent->addObject(o);
				if ( it == objects.end() ) break;
			}
		}
		return;
	}

Problem is, I don't understand why "if ( it == objects.end() ) break;" is needed!

If the objects.erase() returns "end()" the for-loop should catch it before increasing the iterator, why doesn't it?

 

 

That looks wrong to me, if you erase something the iterator is incremented twice. You want to get rid of ++it from the for(;;) line and add else ++it; if the if statement is not entered.


PARTNERS