参照を返すゲッターで範囲for文を回したらバグった

結論

C++では、一時オブジェクトのメンバ変数の参照を、ポインタ変数とか参照変数に代入してはダメ。
とくに範囲for文の範囲表現は、参照変数に代入してからループを回す扱いになるので注意。

バグコード

#include <iostream>
#include <vector>

// vectorのラッパークラス
template<typename T>
class Foo {
  std::vector<T> data_;

public:
  Foo(const std::vector<T>& data) : data_(data) {}

  // コピーのコストを嫌ってconst参照で返すゲッター
  const std::vector<T>& data() const { return data_; }
};

// テンプレート引数を省略するためのファクトリ
template<typename T>
Foo<T> make_foo(const std::vector<T>& data) {
  return Foo<T>(data);
};

int main() {
  std::vector<int> is = {0, 1, 2};
  for (int i : make_foo(is).data()) std::cout << i << ", "; // 何が起こるか分からない！！！
  std::cout << std::endl;
  
  std::vector<int> is2 = make_foo(is).data(); // コピー代入
  for (int i : is2) std::cout << i << ", "; // 大丈夫！！！
  std::cout << std::endl;
  
  return 0;
}

これを実行すると、

12396288, 0, 2, 
0, 1, 2,

となったり、

0, 1, 2,

となったりします。こわい

ちなみに私が実際にハマったバグでは、C++11の範囲for文が使えない残念環境だったので、boostライブラリのBOOST_FOREACHマクロを使っていました。

解説

const参照を返すゲッターは、値のコピーをしなくていい利点の代わりに、ちょっと危なっかしいです。
Effective C++ 第3版 28項「オブジェクト内部のデータへの「ハンドル」を戻さないようにしよう」では危険な例として下記のようなものが挙げられています。

std::vector<int>* ptr = &make_foo(is).data();

make_foo(is)の返り値は一時オブジェクトなため、この行を終えると破棄されてしまうので、そのメンバ変数のポインタが無効になってしまうのですね。
そして、範囲for文でも同様のことが起きてしまっていて、範囲for文がどのように展開されるというと

{
  auto && __range = range_expression; 
  for (auto __begin = begin_expr, __end = end_expr;  __begin != __end; ++__begin) { 
    range_declaration = *__begin; 
    loop_statement
  }
}

というふうに、範囲表現を参照変数に代入する扱いとのことなので、下記のようなコードと同等になって、

{
  const std::vector<int>& range = make_foo(is).data(); 
  for (auto itr = range.begin(), end = range.end();  itr != end; ++itr) { 
    int i = *itr;
    std::cout << i << ", ";
  }
}

範囲表現の中に含まれる一時オブジェクトはループ回っている時にはもう破棄されてしまっている、ということのようです。

愚痴

さて、この手のバグはどう回避すればいいんでしょうね……。
const参照を返すな値を返せ、と言われればそれまでなんですが、やはりコピーのコストが馬鹿にならないことはあります。では呼び出し側で気を付ければいいのかというと、ポインタ変数で受けるのは明らかにおかしいと分かるのですが、範囲for文を回すのは（少なくとも私にとっては）一見問題なさそうなので厄介だなぁ、と。
コンパイラがエラーでも吐いてくれると有難いんですが、そもそもオブジェクトの生存期間って静的には決まらない気がしますし、うーん。。。