0

I have to do a trivial iteration of all the elements in a std::map, first to last. On each element I have to perform an operation. Assume that the map contains the following pairs:

map<string,string> farm={
                        {"one","puppy"},
                        {"two","kitty"},
                        {"three","sheepy"}
                       }

The bit of code that performs the iteration is the following:

for(map<string,string>::iterator beast; beast!=farm.end(); ++beast) 
{
     feed(*beast);
}

Now the surprise is that the above code works for the first element (puppy is fed) but the iterator fails to go to the next elements. The debugger shows that ++beast never returns (seems like it is recursively branching on its left leaf forever).

It seems that the issue here is that beast is never assigned to farm.begin() and as such it can't progress to the next item (see first element of the for).

So here are my questions:

  • Is it normal that the default constructor of a map iterator automatically positions the object to point to the map.begin() element?

  • If that is common practice, then why a valid first element is returned (it could have been returned map.end() for example) ?

  • How could the operator++ be allowed to quietly fail in an infinite loop? Wouldn't be better to return with an error code (we disabled exceptions) or fail openly somehow ?

I wonder if the standard allows this kind of behaviours or it is an implementation choice.

Assumptions: I am not using C++11, I am using Green Hills compiler v2016 with execption support disabled

EDIT:

I am trying to understand why I get a valid value and a quiet fail as in other threads people suggest that a default-constructed iterator is assigned to map.end(). Does the standard gives guidance on this?

  • 2
    Possible duplicate of [What is an iterator's default value?](https://stackoverflow.com/questions/3395180/what-is-an-iterators-default-value) – Gaurav Sehgal Dec 08 '17 at 12:20
  • 4
    Undefined behavior, So expect anything to happen. – PaulMcKenzie Dec 08 '17 at 12:23
  • @GauravSehgal thank you for pointing that post to me because in my case it is happening exactly the opposite of what that post describes (I get a not-incrementable map.begin() instead of a more logical map.end() ), and I am asking what's the logic behind this and why it fails quietely – Giuseppe Cotugno Dec 08 '17 at 12:25
  • Try to use a STL algorithms instead of raw loops. –  Dec 08 '17 at 12:29
  • @Assimazza -- Change compiler options, and the behavior may change again. – PaulMcKenzie Dec 08 '17 at 12:30
  • @manni66 any suggested algorithm? – Giuseppe Cotugno Dec 08 '17 at 12:31
  • @PaulMcKenzie I could escalate the question even more, if it is undefined behaviour then what's the point of allowing a default constructor for iterator – Giuseppe Cotugno Dec 08 '17 at 12:32
  • 2
    @Assimazza C++ allows you to do all sorts of things that are undefined behavior. Declaring an iterator that isn't hooked into a container is valid code. The bug is yours for not doing so, not C++. – PaulMcKenzie Dec 08 '17 at 12:33
  • Try std::for_each –  Dec 08 '17 at 12:34
  • 3
    _what's the point of allowing a default constructor for iterator_ `map::iterator i; if(useA) i = a.begin(); else i = b.begin();` –  Dec 08 '17 at 12:35
  • @PaulMcKenzie I don't question my error, I question why I got a map.beign(), which fooled me around for a few hours, instead of a map.end() which would have pointed me straight to the problem. I think undefined behaviour answers the question, thanks – Giuseppe Cotugno Dec 08 '17 at 12:41
  • 1
    There doesn't exist **a** map::end(). Every map has it's own. You are not allowed to compare iterators originating from different maps. –  Dec 08 '17 at 12:45

1 Answers1

2

Is it normal that the default constructor of a map iterator automatically positions the object to point to the map.begin() element?

No, you should initialize it properly:

for(map<string,string>::iterator beast = farm.begin(); beast!=farm.end(); ++beast)

Btw there is no way that the compiler can know that you want map<string,string>::iterator beast to be an iterator for farm, of course you need to get an iterator from the container and not just create an iterator and assume it points to the container you wish.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
  • The question is not what to do, the question is why it defaulted to a valid value (.begin() ) instead of defaulting to a clearly invalid one (such as .end() ) – Giuseppe Cotugno Dec 08 '17 at 12:43
  • Tobi303 you got exactly why I am so puzzled. How did it know that it should be pointing to farm and yet it got me the first valid pair? Maybe there is some other issue here.... – Giuseppe Cotugno Dec 08 '17 at 12:47
  • @Assimazza "The question is..." you should ask one question per question. I concentrated on the first, as the others just follow from a wrong assumption that in the presence of Undefined behaviour (incrementing an uninitialized iterator) one could reason about what is going on – 463035818_is_not_an_ai Dec 08 '17 at 12:58
  • A proper initialisation fixes the problem with no side effects, but I am really intrigued on how the heck it is possible for a default constructed iterator to point to the first element of a collection never seen before. Some memory black magic possibly – Giuseppe Cotugno Dec 08 '17 at 13:02
  • 2
    @Assimazza dont overthink it, it is plain UB nothing more. You are simply not allowed to do `++beast` if `beast` is uninitialized. The compiler notices that you broke the rules and can do whatever he feels like – 463035818_is_not_an_ai Dec 08 '17 at 13:11
  • 1
    It did not "default to a valid value". A default-constructed iterator has a **singular** value; it cannot be dereferenced nor incremented. Code that does either of those has undefined behavior -- you cannot infer anything meaningful from doing that. – Pete Becker Dec 08 '17 at 14:08