Jump to content
Arvid P.

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

Recommended Posts

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!

Share this post


Link to post
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.

Share this post


Link to post

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?

Share this post


Link to post

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....

 

Share this post


Link to post
Posted (edited)
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 by Remy Lebeau
  • Like 1

Share this post


Link to post

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.

Share this post


Link to post

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! 

Share this post


Link to post
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());
}

 

Share this post


Link to post

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.

Share this post


Link to post

Note that iterators have a _Ptr member. You can null test that.  Works perfect!

 

 

Share this post


Link to post
6 hours ago, Harry Bego said:

Note that iterators have a _Ptr member. You can null test that.  Works perfect!

That is an implementation detail, don't rely on that.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×