37

I've got a class with an atomic member variable:

struct Foo
{
  std::atomic<bool> bar;
  /* ... lots of other stuff, not relevant here ... */
  Foo() 
  : bar( false )
  {}

  /* Trivial implementation fails in gcc 4.7 with:
   *   error: use of deleted function ‘std::atomic<bool>::atomic(const td::atomic<bool>&)’
   */
  Foo( Foo&& other )
  : bar( other.bar )
  {}
};

Foo f;
Foo f2(std::move(f));  // use the move

How should be move constructor look like?

Gcc 4.7 doesn't like any of my attempts (like adding std::move() around the other.bar) and the net is surprisingly quiet here...

ildjarn
  • 62,044
  • 9
  • 127
  • 211
Chris
  • 3,265
  • 5
  • 37
  • 50

3 Answers3

37

std::atomic is not copyable or movable because its copy constructor is deleted and no move constructor is defined. You have to explicitly load the other value and use it construct the new value, as it was pointed out in gustaf's answer.

Why is std::atomic not movable? As it is a synchronization primitive, all threads have to synchronize on the same data (i.e., the same address). When you copy (or move) an atomic value, you have to use some communication protocol. It may be simple, as in your example (just load it and use it to initialize the new atomic) but, in general, I think it is a good design decision by C++11 to force you to think about it. Otherwise, it may result in code that looks fine, but has some subtile synchronization issues.

Philipp Claßen
  • 41,306
  • 31
  • 146
  • 239
  • 2
    I'm trying to figure out a situation where it could actually be problematic to have an atomic automatically moved when you have multiple threads. Manually forcing a move with std::move() is another thing. Either way, I'd like an example to show where moving an atomic would be problematic. You can move any other `std` container, and you're expected to take care of synchronization. – gustaf r Jan 06 '13 at 17:48
  • 1
    I'd also love to know if it would be a bad idea if `atomic` would have a move constructor that would have to be triggered by explicitly calling `std::move()` - it would have given me an interface that I'd have expected. But I'm too far away from beeing a C++ expert to judge if that decision about the standard has a purpose or is a mistake... – Chris Jan 06 '13 at 18:27
  • 1
    There is no way for the move constructor to decide whether the rvalue resulted from an explicit `std::move` or is an implicitly rvalue (e.g., the return value of a function). I'm no expert on the design decisions either, but I think a reference-counted resource is an example where the move constructor could lead to problems. I'll edit my answer and include the example that I had in mind when I wrote about "synchronization issues". – Philipp Claßen Jan 07 '13 at 00:33
  • 1
    I thought about it again, but frankly I couldn't come up with any example where you want to move an atomic. Even the ref-counted example is flawed because there is never any need to do anything like a move. So I come to the conclusion that you *never* want to logically move it, although sometimes you want to copy it. Copying, however, is more complex than you first think of. You have to load the value (which may involve cache sychronization), and then you have to assign it (i.e., another possible synchronization). Having said that, I think the approach of explicitly requiring `load` is fine. – Philipp Claßen Jan 07 '13 at 04:24
22

Since you're moving other, no one else will access it. So reading from its bar is safe, wether it's atomic or not.

atomic<T> only has two constructors, one being the default (), the other being (T). So, your code looks like it should compile. If it doesn't, what happens if you static_cast other.bar to T, enforcing the (T) constructor to be used?

: bar( static_cast< bool >( other.bar ) )

or which is equal to, and perhaps less ugly:

: bar( other.bar.load( ) )

gustaf r
  • 1,224
  • 9
  • 14
  • 1
    Thanks, the `bar( other.bar.load() )` is the right solution that's compiling now! – Chris Jan 06 '13 at 13:16
  • 8
    _So, your code looks like it should compile._ No, `atomic` has a deleted copy constructor and overload resolution finds that not the `atomic(T)` constructor. The cast or load is necessary. – Jonathan Wakely Jan 06 '13 at 13:16
  • @JonathanWakely you're right, I thought the compiler would continue to the next possible constructor `(T)` when it knows the copy constructor is deleted and there is an `operator T`. Is the reason for this that the atomic constructor for `(T)` is `constexpr` making it behave similarly to `explicit` constructors in that implicit conversions must be either explicit (or in this case `constexpr`)? – gustaf r Jan 06 '13 at 17:39
  • 5
    No, the reason is that the best match for the call is the copy constructor, so overload resolution chooses that. A deleted function does **not** mean "ignore this function" it means "if this function would be called your program fails to compile" – Jonathan Wakely Jan 06 '13 at 17:43
  • This doesn't work when Foo has other member variables. One would have to painstakingly move every single one of them in the user defined move constructor. I ran into this several times and have thus decided to avoid atomics whenever I can and use mutexes instead. – John Jiang Nov 21 '19 at 18:08
3

The template instantiation of atomic<bool> essentially looks like this:

struct atomic<bool>
{
    atomic<bool>(bool);
    atomic<bool>( const atomic<bool>& ) = delete;
    operator bool() const;
}

So when you try to copy it:

atomic<bool> a = ...;
atomic<bool> b(a);

The deleted copy constructor is chosen and causes a compile error.

You need to explicitly cast to bool to go through operator bool() --> atomic<bool>(bool)...

atomic<bool> a = ...;
atomic<bool> b(bool(a));
Andrew Tomazos
  • 66,139
  • 40
  • 186
  • 319
  • 1
    Isn't it safer to use static_cast (referring to your last line of code in your solution)? – Guy Avraham Jun 04 '17 at 06:47
  • @GuyAvraham it is as new named casts are preferred for clarity reasons over older syntax. In this case `.load()` would bring the most clarity. – Xeverous Nov 26 '20 at 13:54