NULL iterators in C++ Builder - 32 bit vs. 64 bit

[Arvid P. 0]

Hello community,

 

this is driving me crazy, maybe someone can give me some insight. I have a project originally developed in C++ Builder 2007, that was recently converted to C++ Builder 12, which was already a lot of work. The next step wuld be to turn it from 32 bit into a 64 bit program. Of course, new compiler and linker errors appear and code has to be adapted. But this one really puzzles me:

We have some classes that are wrappers for a list of other classes and have a publicly declared iterator for that list. This iterator was intialized as NULL, sometimes set to NULL and checked against NULL. This worked perfectly fine in 32 bit. But compiling in 64 bit gives linker errors  like this:

 

Quote

no matching constructor for initialization of 'std::list<CSiteOrderItem>::iterator' (aka '_List_iterator<_List_val<std::_List_simple_types<MyType> > >')

 

invalid operands to binary expression ('std::list<CSiteOrderItem>::iterator' (aka '_List_iterator<_List_val<std::_List_simple_types<MyType> > >') and 'int')

 

no viable overloaded '='

 

I did a lot of Google searching and everywhere it says setting iterators to NULL is absolutely not possible in C++. But nowhere I find reference to it being possible in C++ Builder which it definitley is - just not in 64 bit, apparently. Can anyone confirm that that is the case? And maybe give some hints how I can solve the issue without completely rewriting these classes? If it is at all possible...

 

Thanks a lot in advance!

[Remy Lebeau 1269]

10 minutes ago, Arvid P. said:

We have some classes that are wrappers for a list of other classes and have a publicly declared iterator for that list. This iterator was intialized as NULL, sometimes set to NULL and checked against NULL. This worked perfectly fine in 32 bit. But compiling in 64 bit gives linker errors

Can you show the actual code?  Standard iterators are not initializable/comparable with NULL values.  So your code is likely wrong to begin with and just happened to work before due to implementation details of the old standard library, but the new standard library is exposing your bugs.

10 minutes ago, Arvid P. said:

Can anyone confirm that that is the case? And maybe give some hints how I can solve the issue without completely rewriting these classes? If it is at all possible...

You need to fix your buggy code to use iterators correctly.  But it's hard to explain what you need to change until we see what you are actually doing.

[Arvid P. 0]

Hi Remy, thanks for the quick reply and sorry for the late reaction. Let me give you a generalized example.

So let's say there's a class called 

MyType

and another called 

MyTypes

In the header of MyTypes is defined as private:

std::list<MyType*> mLstMyTypes;
std::list<MyType*>::const_iterator mp1MyType;

MyTypes has public methods like:

void MyTypes::first()
{
	mp1MyType = NULL;
}

bool MyTypes::next()
{
	if ( mp1MyType == NULL )
	{
		mp1MyType = mLstMyTypes.begin();
	}
	else
	{
		mp1MyType++;
	}

	if ( mp1MyType == mLstMyTypes.end() )
	{
		mp1MyType = NULL;
		return false;
	}
	else
	{
		return true;
	}
}

MyType* MyTypes::getPresent() const
{
	if ( mp1MyType == NULL )
	{
		return NULL;
	}
	else
	{
		return *mp1MyType;
	}
}

 

I would use these in other places to iterate through the list (that was created from a SQL query when creating the instance of MyTypes).

 

Somehow, this used to work in 32 bit, although it apparently shouldn't. I wasn't aware of that until I started researching the reason for these errors. What can be done?

[Roger Cigol 89]

just add a bool variable to keep track of if mp1MyType is valid or not ? and when not valid (equivalent to your old NULL value) simply ignore the value of mp1MyType ? seems simple enough to fix the code snippet you posted....

 

[Remy Lebeau 1269]

8 hours ago, Arvid P. said:

void MyTypes::first()
{
	mp1MyType = NULL;
}

 

That is absolutely the wrong thing to do.  There is no such concept as a "null iterator" in the standard library.  An iterator has to always point at something valid, whether it is the 1st item in a container/range, the end of a container/range, or anything in between.

 

For what you are attempting, you would need to wrap the iterator inside a std::optional (or equivalent, like Roger Cigol suggested), eg:

std::optional<std::list<MyType*>::const_iterator> mp1MyType;

void MyTypes::first()
{
	mp1MyType = std::nullopt();
}

bool MyTypes::next()
{
	if ( !mp1MyType.has_value() )
		mp1MyType = mLstMyTypes.begin();
	else
		++(*mp1MyType);

	if ( *mp1MyType == mLstMyTypes.end() )
	{
		mp1MyType = std::nullopt();
		return false;
	}

	return true;
}

MyType* MyTypes::getPresent() const
{
	return mp1MyType.has_value() ? *(*mp1MyType) : nullptr;
}

But, why are you waiting until next() is called to move the iterator to the 1st item?  Why can't you do it in first() instead?  Seems like first() should return a bool indicating whether the iterator is valid, just like next() does, eg:

std::list<MyType*>::const_iterator mp1MyType;

bool MyTypes::first()
{
	mp1MyType = mLstMyTypes.begin();
	return ( mp1MyType != mLstMyTypes.end() );
}

bool MyTypes::next()
{
	if ( mp1MyType != mLstMyTypes.end() )
	{
		++mp1MyType;
		if ( mp1MyType != mLstMyTypes.end() )
			return true;
	}
	return false;
}

MyType* MyTypes::getPresent() const
{
	return ( mp1MyType != mLstMyTypes.end() ) ? *mp1MyType : nullptr;
}

Edited May 2 by Remy Lebeau

[bdw_nz20 11]

Yes I think Remy's second example would be a nice solution and actually how I do it for an abstract base class with code implemented then for specific data types i.e. different file formats and sql.

[Arvid P. 0]

Thanks everyone for your suggestions. Using an optional as a wrapper for the iterator sounds like a great idea! I'll try that and let you know how I fared.

In my defense, this is a very old project and I'm not the original developer. This concept was used in some classes and I used it for others that I added. So unfortunately I'll have to change quite a lot of classes since no inheritance was used. I guess I'll fix that too.

Thanks again, you were a great help! 

[Remy Lebeau 1269]

11 hours ago, Arvid P. said:

In my defense, this is a very old project and I'm not the original developer. This concept was used in some classes and I used it for others that I added. So unfortunately I'll have to change quite a lot of classes since no inheritance was used. I guess I'll fix that too.

The original code you showed makes sense if the calling code is using this kind of pattern:

first();
while (next()) {
	// do something with getPresent() ...
}

Whereas my 2nd example would need this kind of pattern instead:

if (First()) {
	do {
		// do something with getPresent() ...
	}
	while (Next());
}

 

[Arvid P. 0]

Right, the first pattern ist what is used in the program. I didn't have the time yet to try the optional approach since the 64 bit conversion isn't the highest priority right now in the project. I have to talk with my project manager about it. But I think that's the way to go.