Sign in to follow this  

C++ Compiler Error

Recommended Posts

powerskill    122
//the following code gives mad compiler errors //it says there should be a ; befor the it
template <class map> 
inline void DeleteSTLMap(map& m) 
      for (map::iterator it = m.begin(); it!=m.end(); ++it) 
      { delete it->second; it->second = NULL; } 
Dev-Cpp\ai\misc\Utils.h In function `void DeleteSTLContainer(container&)': Dev-Cpp\ai\misc\Utils.h expected `;' before "it" Dev-Cpp\ai\misc\Utils.h `it' undeclared (first use this function)

Share this post

Link to post
Share on other sites
mmelson    232
I believe that you need to make that

for(typename map::iterator ...)

However, this is one of those things that I'd have to test before I'm 100% sure I'm right, but I don't have access to a compiler right at the moment, so if I doesn't work, sorry. =P

Share this post

Link to post
Share on other sites
Muhammad Haggag    1358
Templates are compiled in 2 passes or phases. First when the compiler encounters the code definition/declaration, and second when the compiler instantiates the code for a specific template parameter (i.e. for a specific type substituted for map).

The error comes from this:

In the first pass of compilation, the compiler does not know what map refers to, since it's not instantiating the function yet. As such, the expression above could be seen as a declaration (if map::iterator were a type), or a member variable access (if iterator were a member variable). So you get an error because the compiler can't decide.

To solve this, you have to use the typename keyword for disambiguation. Basically:
for (typename map::iterator it = m.begin(); it!=m.end(); ++it)

This tells the compiler that map::iterator is actually a declaration (i.e. iterator is a type). A more thorough explanation can be found here.

Share this post

Link to post
Share on other sites
Zahlman    1682
You might also want to reconsider the name for your template parameter (since it's preventing you from doing e.g. 'using std::map' in the same scope) :)

Of course, this is a workaround for a bad design, anyway, really. Apparently you are storing a map of (something) to pointer-to-(something), where each of those pointers-to-something "owns" the pointed-at thing (such that you know you can safely 'delete' via that pointer). A better way to handle this situation is to make a wrapper (or use a pre-built one, such as provided by Boost) for the pointer, along the lines of:

template <class pointedAt>
class OwningPointer {
pointedAt* owned;

// We make it so that copying the "pointer" copies the pointed-at thing, as
// well; this implies that we should clean up the pointed-at thing when the
// pointer dies, and make a copy of the pointed-at thing when we assign from
// another pointer (so that each pointer owns a separate thing).
// Because we had to do that cleanup for the destructor (to avoid memory leaks
// for general use), we find that we never need to worry about cleaning up
// OwningPointers stored in a map. The memory management work is properly
// organized and delegated: the map cleans up its allocations, and the things
// that were stored in the map clean up their allocations, and no "janitor"
// code is needed.
OwningPointer(pointedAt* toOwn): owned(toOwn) {}
OwningPointer(const OwningPointer& other): owned(new pointedAt(other.toOwn)) {}
OwningPointer& operator=(const OwningPointer& rhs) {
OwningPointer other(rhs);
std::swap(owned, other.owned);
return *this;
~OwningPointer() {
delete owned;

// Of course, we also want to make our "pointer" behave like a pointer:
pointedAt& operator*() { return *owned; }
pointedAt* operator->() { return owned; }

Now we can just use OwningPointer<T> for the map value-type, instead of T*, and all is well.

(As written, this does not properly handle the case where T needs to behave polymorphically, because the base class' copy constructor will always be used. However, this can be worked around - see the 'virtual clone idiom'.)

Alternatively, you could just use boost::ptr_map and save yourself the trouble? :)

Share this post

Link to post
Share on other sites

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

Sign in to follow this