Object lifetime issue with range-based for loop

Question

I'm currently converting my code to C++11 and having trouble with the following range-based for loop (FWIW, the same code ran just fine before with BOOST for_each).

The mLibraryFiles member (STL vector) is part of a singleton instance and guaranteed to exist after the method returns, however when the caller inspects the returned object it contains only garbage.

Debugging through it appears to turn from expected content to garbage just after returning from the method so I suppose I must have gotten something wrong about how the C++11 range-based loop actually works:

Entry* FindEntry(string inName)
{  Entry *rs = NULL;

   for (auto libraryEntry : mLibraryFiles)
   {
      if (libraryEntry.filename.compare(inName) == 0)
      {
         rs = &libraryEntry;
         break;
      }
   }

   return rs;
}

I would have expected that the libraryEntry variable represents the actual objects in the mLibraryFiles vector?

Any insights much appreciated!
(compiler is LLVM 5.0 though I doubt this matters..)

`libraryEntry` is a *copy* of the element of `mLibraryFiles`. Use `for (auto& libraryEntry : mLibraryFiles)` instead. — Casey, Jan 18 '14 at 17:08
llvm is not currently at version 3.4. You probably mean clang? — Nil, Jan 18 '14 at 17:09
@Nil Apple uses a separate version numbering scheme for their LLVM releases. (And yes, [LLVM is currently at version 3.4](http://llvm.org/releases/3.4/docs/ReleaseNotes.html).) Apple's releases don't necessarily coincide with upstream LLVM releases, so they use a different numbering scheme to "avoid confusion." — Casey, Jan 18 '14 at 17:09
@Nil Yes, sorry for the confusion - version as shown in Xcode 5, which states *Apple LLVM 5.0* — Jay, Jan 18 '14 at 17:11
possible duplicate of [C++11 range based loop: get item by value or const reference](http://stackoverflow.com/questions/15176104/c11-range-based-loop-get-item-by-value-or-const-reference) — Casey, Jan 18 '14 at 17:13
And since I'm here typing stuff, I tell you that (assuming `libraryEntry.filename` is a `std::string`) you can replace `libraryEntry.filename.compare(inName) == 0` with the equivalent `libraryEntry.filename == inName`. — Casey, Jan 18 '14 at 17:16
@Casey True - thanks for pointing that out! Still way too much `char[]` code in this project waiting to be 'upgraded' to actual objects.. — Jay, Jan 18 '14 at 17:19

score 8 · Accepted Answer · answered Jan 18 '14 at 17:07

8

The values from your vector are copied to libraryEntry in the loop, so you have a pointer to copied value which no longer exists after the loop. Use auto& libraryEntry.

answered Jan 18 '14 at 17:07

Wojtek Surowka

20,535
4
44
51

score 4 · Answer 2 · answered Jan 18 '14 at 17:07

4

Declare libraryEntry as auto& otherwise its a temporary copy which lifetime is bound to the for-loop. Thats why I prefer to write the actual type of the object instead of auto

answered Jan 18 '14 at 17:07

Sebastian Hoffmann

11,127
7
49
77

herb sutter says use auto whenever you can and avoid using the actual type. It gives you the flexibility of typedef for free. – Ezra Jan 18 '14 at 17:16
2

Writing the actual type, also without a `&`, will be just as bad. – Yakk - Adam Nevraumont Jan 18 '14 at 17:18

Object lifetime issue with range-based for loop

2 Answers2