0

I wrote this code:

#define VECTOR_LOOP_V(X) for (vector<typeof(X)>::iterator it = X.begin(); it != X.end(); it++)

to make it faster to write for loops for vectors but for some reason it does not work, and when I try to compile it it gives me really, really long error message.

 test.cpp: In function ‘int main(int, char**)’:
test.cpp:20:5: error: conversion from ‘std::vector<std::basic_string<char> >::iterator {aka __gnu_cxx::__normal_iterator<std::basic_string<char>*, std::vector<std::basic_string<char> > >}’ to non-scalar type ‘std::v

etc..

triclosan
  • 5,578
  • 6
  • 26
  • 50
user1507133
  • 473
  • 1
  • 5
  • 8
  • 8
    Trust me, for C++, that is a __really, really short__ error message. – orlp Jul 20 '12 at 20:18
  • 1
    You should write the complete error message, not just arbitrarily cut somewhere. I bet the first characters cut out say something like `std::vector< std::vector< std::basic_string...` which should give you a hint on what the left hand side of the copy construction is... – David Rodríguez - dribeas Jul 20 '12 at 20:19
  • 3
    C++11 not available to you, I guess? Range-based `for` loops are the new hotness. – Chad Jul 20 '12 at 20:19
  • 1
    And if you don't have range-based `for` yet, try Boost FOREACH. – Fred Larson Jul 20 '12 at 20:23

2 Answers2

4
#define VECTOR_LOOP_V(X) \
     for (vector<typeof(X)>::iterator it = X.begin(); \
          it != X.end(); it++)

How do you intend on using the macro? It looks like X is the container, in which case, the first part of the for should be something like typeof(X)::iterator (don't even know if that is legal, since typeof is not standard and have never used).

Alternatively, you can just use boost::foreach which provides a similar although a bit richer macro, with the advantage that it is safer in many respects. For example, your macro will break awfully if the argument is a function call that returns by value:

std::vector<int> f();
VECTOR_LOOP_V( f() ) {
   std::cout << *it << "\n";
}

The problem is as with almost all macros that you evaluate the arguments multiple times, and in your case it will be an iterator into one std::vector<int> and termination condition will try to compare it with an iterator from a different std::vector<int>.

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
3

It doesn't work because typeof(X) yields std::vector type and so the macro expands to something like:

for (vector<std::vector<...> >::iterator it = X.begin(); it != X.end(); it++)

Also, your macro would fail if X is constant, in which case const_iterator should be used. There are tons of other problems, but at any rate, what you were trying to do is something like this:

#define VECTOR_LOOP_V(X) \
    for (typeof(X.begin()) it = X.begin(), eit = X.end(); it != eit; ++it)

.. and here is some usage example:

#include <vector>
#include <iostream>

#define VECTOR_LOOP_V(X) \
    for (typeof(X.begin()) it = X.begin(), eit = X.end(); it != eit; ++it)

int main()
{
    std::vector<int> v;
    v.push_back(1);
    v.push_back(2);
    v.push_back(3);
    v.push_back(4);
    v.push_back(5);
    VECTOR_LOOP_V(v) {
        std::cout << *it << std::endl;
    }
}

Note, however, that typeof is not a standard C++ (see this Q/A).

All in all, you are better off using BOOST_FOREACH (or at least see how it is implemented there) or C++11 range-based for loop.

P.S.: Do not use the STL abbreviation unless you really mean STL and not C++ Standard Library (which you can refer to as, say, stdlib).

Community
  • 1
  • 1