パラメータがポインタ型のとき、std::stack.pushが意図した通りの動作にならない

Question

以下のコードにて、int型のパラメータを指定すれば987...0と出力されるところ、ポインタ型をしたところt.push(&num)が意図した通りに動作せず999...9と出力されてしまいました。

そこで質問があります

1. numの寿命はforブロック内で終わるが、この記述に問題はないか
2. 恐らくコンパイラの最適化でnumがforの外で宣言され、tにすべて同じアドレスがpushされるためこのようなことが起こっているのだと思われるが、numをそれぞれ別のアドレスに定義し、tの寿命までこれを保持する方法はあるか

可能な限りint*型のままで実装したいのです。
解決策をご教授ください。

開発環境:OS Windows10Home コンパイラ Clang6.0.0

#include <iostream>                
#include <stack>                   
using namespace std;               
                                   
int main()  {                      
▸---stack<int*> t;                 
                                   
▸---for (int i = 0; i < 10; i++) { 
▸---▸---int num = i;              
▸---▸---t.push(&num);              
▸---}                              
▸---while (!t.empty()) {           
▸---▸---cout << *t.top() << endl;  
▸---▸---t.pop();                   
▸---}                              
}                                  

Answer 1

1. numの寿命はforブロック内で終わるが、この記述に問題はないか

大問題です。numは、forの繰り返しごとに生成され、その繰り返しが終わったら廃棄されます。つまりi==0のとき、numが生成され、i==0の繰り返しが終わるときに廃棄され、i==1の時にまた生成され...となります。廃棄された変数のアドレスを参照するのは未定義の動作なので、たとえばi==0のあとi==1の時にプログラムがクラッシュしても「C++の仕様だから」の一言で済ませられることです。

実際に起きているのは、numが都度廃棄されてメモリが空くので、そのメモリを再利用して新たなnumを生成しているのでしょう。

numをそれぞれ別のアドレスに定義し、tの寿命までこれを保持する方法はあるか

int型のメモリを確保する必要があります。生のnewを使うと、popするたびに自分でdeleteしなければならないので、std::unique_ptrかstd::shared_ptrを使うのが普通でしょう。

unique_ptrを使った時のサンプルは

#include <iostream>                
#include <stack>                   
#include <memory>
using namespace std;

int main() {
    stack<unique_ptr<int> > t;

    for (int i = 0; i < 10; i++) {
        auto pnum{ make_unique<int>(i) };
        t.push(std::move(pnum));
    }
    while (!t.empty()) {
        cout << *t.top() << endl;
        t.pop();
    }

    return 0;
}

Comments

[asobinin]

なるほど、ヒープメモリであればこの条件が満たせますね！
ベストアンサーですが、make_uniqueを使った実装のほうが良いと感じたのでこちらを採用させていただきます。

Answer 2

こんにちは。

そのnumはforブロックが終了したタイミングで破棄されますので、そこへのポインタは「あらぬメモリ」を指すダングリング・ポインタになります。
回避するには、newで確保して、pop()する際にdeleteすることが考えられます。

#include <iostream>                
#include <stack>                   
using namespace std;               

int main()  {                      
    stack<int*> t;

    for (int i = 0; i < 10; i++) {
        //int num = i;
        t.push(new int{i});
    }
    while (!t.empty()) {
        cout << *t.top() << endl;
        delete t.top();
        t.pop();
    }  
}

しかし、これはメモリ・リークしやすいので、int*のままという条件を満たしませんが、可能ならばstd::unique_ptrで実装した方が良いです。（std::unique_ptrではだめな場合はstd::shared_ptrを検討下さい。）

#include <iostream>                
#include <stack>                   
#include <memory>
using namespace std;               

int main()  {                      

    stack<unique_ptr<int>> t;

    for (int i = 0; i < 10; i++) {
        //int num = i;
        t.emplace(new int{i});
    }
    while (!t.empty()) {
        cout << *t.top() << endl;
        t.pop();
    } 
}