Disturbing: const QString &

**magland** · 16th November 2007, 19:20

Thanks, Jacek. I put in those type of debug strings as you suggested (code is at the bottom)... and now I'm really starting to be worried of a fundamental bug in the compiler.

The "path" variable changes right in the middle of the function, during a call to a completely different function. I should mention that the "update_node()" function is in a plugin library, and the implementation of the offending "addFile()" is in the main application.

Here's the log that I get out, using the two scenarios. Notice that the "path" variable changes right between debug 5 and debug 6.

Qt Code:

Switch view

LOG OUTPUT SCENARIO #1
...
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qwidget" 
1 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
0 update_node:::::10: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtcore/qdebug" 
...
 
 
LOG OUTPUT SCENARIO #2
...
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qapplication" 
1 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
0 update_node:::::10: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qwidget" 
...

LOG OUTPUT SCENARIO #1
...
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qwidget" 
1 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
0 update_node:::::10: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtcore/qdebug" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtcore/qdebug" 
...


LOG OUTPUT SCENARIO #2
...
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qapplication" 
1 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
1 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication.h" 
0 update_node:::::10: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::11: "c:/qt/4.3.2/include/qtgui/qapplication" 
0 update_node:::::1: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::2: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::3: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::4: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::5: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::6: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::7: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::8: "c:/qt/4.3.2/include/qtgui/qwidget" 
0 update_node:::::9: "c:/qt/4.3.2/include/qtgui/qwidget" 
...

To copy to clipboard, switch view to plain text mode

And here's the code that produced these two (only difference is in the top three lines):

CODE FOR SCENARIO #1

Qt Code:

Switch view

void update_node(const QString &path) {
		static int level=0;
		//QString path=path_in; //apparently this is important :(
		qDebug() << level << "update_node:::::1:" <<path;
		if (!nodes.contains(path)) return;
		if (!nodes[path].needs_to_be_updated) return;
		nodes[path].needs_to_be_updated=false;
		nodes[path].children.clear();
		QList<HaiQTag> tags=core->tagManager->byPath(path);
		emit file_tags_updated(path,tags);
		QStringList search_paths=include_search_paths;
		search_paths << QFileInfo(path).path();
		QStringList included_paths;
		qDebug() << level << "update_node:::::2:" <<path;
		for (int j=0; j<tags.count(); j++) {
			if (tags[j].tagtype=="pound_include") {
				QString holdstr=get_include_file_path(tags[j].signature,search_paths);;
				if (!holdstr.isEmpty())
					included_paths << holdstr;
			}
		}
		qDebug() << level << "update_node:::::3:" <<path;
		for (int j=0; j<included_paths.count(); j++) {
			QString path2=included_paths[j];
			qDebug() << level << "update_node:::::4:" <<path;
			if (!nodes.contains(path2)) {
				qDebug() << level << "update_node:::::5:" <<path;
				core->fileManager->addFile(path2);
				qDebug() << level << "update_node:::::6:" <<path;
				visible_files_node newnode;
				newnode.path=path2;
				newnode.needs_to_be_updated=true;
				nodes[path2]=newnode;
			}
			qDebug() << level << "update_node:::::7:" <<path;
			if ((!nodes[path].children.contains(path2))&&(path!=path2)) {
				nodes[path].children << path2;
			}
			qDebug() << level << "update_node:::::8:" <<path;
			if (nodes[path2].needs_to_be_updated) {
				qDebug() << level << "update_node:::::9:" <<path;
				level++;
				update_node(path2); 
				level--;
				qDebug() << level << "update_node:::::10:" <<path;
			}
		}
		qDebug() << level << "update_node:::::11:" << path;
	}

void update_node(const QString &path) {
		static int level=0;
		//QString path=path_in; //apparently this is important :(
		qDebug() << level << "update_node:::::1:" <<path;
		if (!nodes.contains(path)) return;
		if (!nodes[path].needs_to_be_updated) return;
		nodes[path].needs_to_be_updated=false;
		nodes[path].children.clear();
		QList<HaiQTag> tags=core->tagManager->byPath(path);
		emit file_tags_updated(path,tags);
		QStringList search_paths=include_search_paths;
		search_paths << QFileInfo(path).path();
		QStringList included_paths;
		qDebug() << level << "update_node:::::2:" <<path;
		for (int j=0; j<tags.count(); j++) {
			if (tags[j].tagtype=="pound_include") {
				QString holdstr=get_include_file_path(tags[j].signature,search_paths);;
				if (!holdstr.isEmpty())
					included_paths << holdstr;
			}
		}
		qDebug() << level << "update_node:::::3:" <<path;
		for (int j=0; j<included_paths.count(); j++) {
			QString path2=included_paths[j];
			qDebug() << level << "update_node:::::4:" <<path;
			if (!nodes.contains(path2)) {
				qDebug() << level << "update_node:::::5:" <<path;
				core->fileManager->addFile(path2);
				qDebug() << level << "update_node:::::6:" <<path;
				visible_files_node newnode;
				newnode.path=path2;
				newnode.needs_to_be_updated=true;
				nodes[path2]=newnode;
			}
			qDebug() << level << "update_node:::::7:" <<path;
			if ((!nodes[path].children.contains(path2))&&(path!=path2)) {
				nodes[path].children << path2;
			}
			qDebug() << level << "update_node:::::8:" <<path;
			if (nodes[path2].needs_to_be_updated) {
				qDebug() << level << "update_node:::::9:" <<path;
				level++;
				update_node(path2); 
				level--;
				qDebug() << level << "update_node:::::10:" <<path;
			}
		}
		qDebug() << level << "update_node:::::11:" << path;
	}

To copy to clipboard, switch view to plain text mode

CODE FOR SCENARIO #2

Qt Code:

Switch view

void update_node(const QString &path_in) {
		static int level=0;
		QString path=path_in; //apparently this is important :(
		qDebug() << level << "update_node:::::1:" <<path;
		if (!nodes.contains(path)) return;
		if (!nodes[path].needs_to_be_updated) return;
		nodes[path].needs_to_be_updated=false;
		nodes[path].children.clear();
		QList<HaiQTag> tags=core->tagManager->byPath(path);
		emit file_tags_updated(path,tags);
		QStringList search_paths=include_search_paths;
		search_paths << QFileInfo(path).path();
		QStringList included_paths;
		qDebug() << level << "update_node:::::2:" <<path;
		for (int j=0; j<tags.count(); j++) {
			if (tags[j].tagtype=="pound_include") {
				QString holdstr=get_include_file_path(tags[j].signature,search_paths);;
				if (!holdstr.isEmpty())
					included_paths << holdstr;
			}
		}
		qDebug() << level << "update_node:::::3:" <<path;
		for (int j=0; j<included_paths.count(); j++) {
			QString path2=included_paths[j];
			qDebug() << level << "update_node:::::4:" <<path;
			if (!nodes.contains(path2)) {
				qDebug() << level << "update_node:::::5:" <<path;
				core->fileManager->addFile(path2);
				qDebug() << level << "update_node:::::6:" <<path;
				visible_files_node newnode;
				newnode.path=path2;
				newnode.needs_to_be_updated=true;
				nodes[path2]=newnode;
			}
			qDebug() << level << "update_node:::::7:" <<path;
			if ((!nodes[path].children.contains(path2))&&(path!=path2)) {
				nodes[path].children << path2;
			}
			qDebug() << level << "update_node:::::8:" <<path;
			if (nodes[path2].needs_to_be_updated) {
				qDebug() << level << "update_node:::::9:" <<path;
				level++;
				update_node(path2); 
				level--;
				qDebug() << level << "update_node:::::10:" <<path;
			}
		}
		qDebug() << level << "update_node:::::11:" << path;
	}

void update_node(const QString &path_in) {
		static int level=0;
		QString path=path_in; //apparently this is important :(
		qDebug() << level << "update_node:::::1:" <<path;
		if (!nodes.contains(path)) return;
		if (!nodes[path].needs_to_be_updated) return;
		nodes[path].needs_to_be_updated=false;
		nodes[path].children.clear();
		QList<HaiQTag> tags=core->tagManager->byPath(path);
		emit file_tags_updated(path,tags);
		QStringList search_paths=include_search_paths;
		search_paths << QFileInfo(path).path();
		QStringList included_paths;
		qDebug() << level << "update_node:::::2:" <<path;
		for (int j=0; j<tags.count(); j++) {
			if (tags[j].tagtype=="pound_include") {
				QString holdstr=get_include_file_path(tags[j].signature,search_paths);;
				if (!holdstr.isEmpty())
					included_paths << holdstr;
			}
		}
		qDebug() << level << "update_node:::::3:" <<path;
		for (int j=0; j<included_paths.count(); j++) {
			QString path2=included_paths[j];
			qDebug() << level << "update_node:::::4:" <<path;
			if (!nodes.contains(path2)) {
				qDebug() << level << "update_node:::::5:" <<path;
				core->fileManager->addFile(path2);
				qDebug() << level << "update_node:::::6:" <<path;
				visible_files_node newnode;
				newnode.path=path2;
				newnode.needs_to_be_updated=true;
				nodes[path2]=newnode;
			}
			qDebug() << level << "update_node:::::7:" <<path;
			if ((!nodes[path].children.contains(path2))&&(path!=path2)) {
				nodes[path].children << path2;
			}
			qDebug() << level << "update_node:::::8:" <<path;
			if (nodes[path2].needs_to_be_updated) {
				qDebug() << level << "update_node:::::9:" <<path;
				level++;
				update_node(path2); 
				level--;
				qDebug() << level << "update_node:::::10:" <<path;
			}
		}
		qDebug() << level << "update_node:::::11:" << path;
	}

To copy to clipboard, switch view to plain text mode

and here is the declaration of addFile...

Qt Code:

Switch view

void addFile(const QString &path);

void addFile(const QString &path);

To copy to clipboard, switch view to plain text mode

**jacek** · 16th November 2007, 23:47

Originally Posted by magland

The "path" variable changes right in the middle of the function, during a call to a completely different function.

This means that we have to take a look at this completely different function. It isn't time for worrying yet.

Probably your code does something similar to this:

Qt Code:

Switch view

#include <iostream>
 
int z = 0;
 
void bar( int y )
{
   z = y;
}
 
void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}
 
int main()
{
   foo( z );
   return 0;
}

#include <iostream>

int z = 0;

void bar( int y )
{
   z = y;
}

void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}

int main()
{
   foo( z );
   return 0;
}

To copy to clipboard, switch view to plain text mode

**magland** · 17th November 2007, 12:09

Ok, I see what you are saying. Now I see the danger of passing things by reference (even if declared const). The advantage is that the parameter does not need to be copied, but the tradeoff is that you need to worry about whether the variable is changed part way through the execution of the function - not within the function but elsewhere.

This is probably what was happening to me. But I will never know, because I've restructured my program to avoid the recursion (I'm now using a queuing system to do the same task).

Based on a recent recommendation on this forum I changed my coding habits and use "const QString &" everywhere instead of "QString".... but now I see that I need to be more careful. Do you have a recommendation of what precautions I need to take with this?

Thanks!

Originally Posted by jacek

This means that we have to take a look at this completely different function. It isn't time for worrying yet.

Probably your code does something similar to this:

Qt Code:

Switch view

#include <iostream>
 
int z = 0;
 
void bar( int y )
{
   z = y;
}
 
void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}
 
int main()
{
   foo( z );
   return 0;
}

#include <iostream>

int z = 0;

void bar( int y )
{
   z = y;
}

void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}

int main()
{
   foo( z );
   return 0;
}

To copy to clipboard, switch view to plain text mode

**jacek** · 28th November 2007, 01:26

Originally Posted by magland

Based on a recent recommendation on this forum I changed my coding habits and use "const QString &" everywhere instead of "QString".... but now I see that I need to be more careful.

It's recommended to pass objects by reference to avoid copying, but on the other hand in Qt it's cheap to copy implicitly shared classes.

Originally Posted by magland

Do you have a recommendation of what precautions I need to take with this?

I was thinking about this for few days and I couldn't come up with any rules. You situation is a bit similar to:

Qt Code:

Switch view

QVector v;
...
v.append( v[0] );

QVector v;
...
v.append( v[0] );

To copy to clipboard, switch view to plain text mode

The argument is passed as reference, but it must be copied when QVector needs to reallocate data. You can easily identify such situation in a single data structure, but the problem arises when the data is passed between many classes. But I would simply classify that as one of those Bad Things(tm) that might happen when you use global variables (or you have cycles in call graph).

**magland** · 28th November 2007, 16:34

Thanks Jacek. I agree with you.

After finding a number of subtle problems caused by passing by reference (in addition to the one I reported) I have concluded that it is important to NEVER pass arguments by reference UNLESS you can guarantee that the function does not access global variables, or "have cycles in the call graph" that may end up modifying the input variable. It is just not worth the danger! There's nothing worse than problems that occur mysteriously once in every 1000 calls.

Of course, passing by reference is good, if you can ensure such things. I don't use global variables, but I do use a model/view framework which inevitably will have some cycles or recursive calls. Therefore, I need to be careful about it.

**Gopala Krishna** · 20th November 2007, 13:55

Originally Posted by jacek

This means that we have to take a look at this completely different function. It isn't time for worrying yet.

Probably your code does something similar to this:

Qt Code:

Switch view

#include <iostream>
 
int z = 0;
 
void bar( int y )
{
   z = y;
}
 
void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}
 
int main()
{
   foo( z );
   return 0;
}

#include <iostream>

int z = 0;

void bar( int y )
{
   z = y;
}

void foo( const int & x )
{
   std::cerr << x << std::endl;
   bar( 1 );
   std::cerr << x << std::endl;
}

int main()
{
   foo( z );
   return 0;
}

To copy to clipboard, switch view to plain text mode

Very cool observation jacek :-)

So basically in the code posted by magland, it seems that the parameter passed is a class member variable(as there is an emit statement, the func should be a class member) which is modified somewhere may be in a slot.
The actual answer to whether the reference form should be used or not is a dependent question. It depends on whether you modify the parameter sent outside the function when the function with const reference is not yet returned.

Anyway, many thanks Jacek as well as magland for exposing such a kind of potential bug.
A lesson learnt well in the right time

Thread: Disturbing: const QString &

Thread Tools

Search Thread

Display

Hybrid View

Re: Disturbing: const QString &

Re: Disturbing: const QString &

The following 2 users say thank you to jacek for this useful post:

Re: Disturbing: const QString &

The following user says thank you to magland for this useful post:

Re: Disturbing: const QString &

Re: Disturbing: const QString &

Re: Disturbing: const QString &

Similar Threads

qt 4.2.2 install in tru64 cxx

Convert from iso-8859-1 to... Something else :-)

QTableView paints too much

Delegates and Table

[4.1.1] Custom Widget visible in Edit but not in Preview

Bookmarks

Bookmarks

Posting Permissions