C++ bug when using range-based for loop and vector of pointers

Question

While messing around with vectors of pointers, I stumbled upon this weird bug:

mwe.cpp:

#include <iostream>
#include <vector>

class A {
  public:
    int x = 42; // making this unsigned causes output to be only positive
    int y;      // removing this makes bug disappear
};

class B {
  public:
    B(std::vector<A *> pt_vec) {
      std::cout << pt_vec[0]->x << std::endl;
    };
};

int main() {
  std::vector<A *> pt_vec;

  A a;                 //  these 3 lines can
  std::vector<A> vec;  //  be replaced with
  vec.push_back(a);    //  std::vector<A> vec {A()};
                       //  the bug will still appear

  for (A el: vec) {         // doing this without for (or
    pt_vec.push_back(&el);  // with iterators/good ol' for)
  }                         // makes bug disappear

  B b = B(pt_vec);

  //exit(1);    // uncommenting this makes bug disappear
}

How to reproduce:

$ gcc --version
gcc (Gentoo 9.2.0-r2 p3) 9.2.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ g++ -O0 -Wall -Wpedantic mwe.cpp -o mwe
mwe.cpp: In function ‘int main()’:
mwe.cpp:27:5: warning: variable ‘b’ set but not used [-Wunused-but-set-variable]
   27 |   B b = B(pt_vec);
      |     ^

(Bug also reproduced with GCC 9.2.0 on ARCH, but Apple clang 11.0.0 gave the expected output)

Expected output:

$ ./mwe
42

Actual output:

$ ./mwe 
1533476528
$ ./mwe
-1607700816
$ ./mwe
<insert random huge number here>

Do I inadvertently introduce Undefined Behavior somewhere? If not, why doesn't this work as expected?

`for (A el: vec)` - you iterate over *copies* of the elements of vector. Then you take an address of this copy, push_back it into second vector and copy object is discarded. — Yksisarvinen, Nov 13 '19 at 12:03
That duplicate is good, but I vote to close as a simple typographical error. — underscore_d, Nov 13 '19 at 12:08

score 4 · Accepted Answer · answered Nov 13 '19 at 12:04

4

for (A el: vec) means that you are creating a copy of every element in vec while iterating. &el will be a pointer to a object that is going to die at the end of the iteration.

You want to use for (A& el: vec) to iterate over the elements of vec by reference. &el will then be a pointer to the actual elements inside vec.

answered Nov 13 '19 at 12:04

Vittorio Romeo

90,666
33
258
416

I'm curious why this works as I expected with clang. – orfeas Nov 13 '19 at 13:06
1

@orfeas: you have undefined behavior. Anything can happen, including the program appearing to work. – Vittorio Romeo Nov 13 '19 at 13:14

C++ bug when using range-based for loop and vector of pointers

1 Answers1