1

I have a simple piece of code as follows:

#include <map>
#include <iostream>

template <typename LocType, typename Base>
class MapWrapper {
public:
  Base&& get_and_erase(LocType x) {
    Base ret = std::move(_data[x]);
    _data.erase(x);
    // Uncomment the cout will give correct result
    // std::cout << "retval = " << ret << std::endl;
    return std::move(ret);
  }
  void increase(const LocType& x, const Base& w) {
    if (w == 0.0) {
      return;
    }
    _data[x] += w;
  }
private:
  std::map<LocType, Base> _data;
};

int main() {
  MapWrapper<int, double> a;
  a.increase(1, 1.0);
  double w = a.get_and_erase(1);
  std::cout << "w = " << w << std::endl;
  return 0;
}

I think the output should be 1. It works fine in g++ 4.8.2, but when I use my MAC with

Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin14.5.0
Thread model: posix

and compile it with:

g++ --std=c++11 -O2 debug.cpp -o debug

What I get is :

w = 2.64619e-260

The only way I can make it correct is to turn off -O2 or force an output by uncomment the std::cout in the code.

Any ideas?

Mat
  • 202,337
  • 40
  • 393
  • 406
  • Related: http://stackoverflow.com/questions/1116641/is-returning-by-rvalue-reference-more-efficient – MikeMB Jan 18 '16 at 09:40
  • I believe there are very few instances, where an r-valze return type isn't an error let alone necessary / an optimization. – MikeMB Jan 18 '16 at 09:43

1 Answers1

3

Your code has undefined behavior. get_and_erase returns a reference to a local variable. Enabling optimizations exposes this bug. Your explicit std::move fools the warning the compiler would usually emit for returning a reference to a local variable.

To fix this, change the return type to just Base, and the return statement to just return ret;; the move is unnecessary and actually a pessimization.

Sebastian Redl
  • 69,373
  • 8
  • 123
  • 157