Avoid relying on implicit copy constructor/operator deprecated in C++11

[Lastique]

C++11 deprecates implicit default copy constructors and operators if the class has user-defined destructor or copy constructor/operator. gcc 9 generates warnings when this deprecated language feature is used. This commit fixes that by providing user-defained copy constructors/operators where needed. The added definitions are equivalent to the implicitly generated by the compiler.

Additionally, for thread::id added move constructor/operator when rvalue references are available. Move is more efficient when the id uses a smart pointer internally.

[pdimov]

Enumerating members by hand is too error prone for my taste; how about using =default instead?

I'd also split the thread::id change into its own PR. (It seems to break atomic<thread::id>.)

[Lastique]

Enumerating members by hand is too error prone for my taste; how about using =default instead?

That would require C++11, and I wouldn't want to raise the minimum required C++ version for Boost.Thread. Using it conditionally is not useful as I would still have to provide the C++03 version.

I'd also split the thread::id change into its own PR.

It's related, because id has the same problem - it has a copy constructor but not operator=.

(It seems to break atomic<thread::id>.)

I believe, it never was valid because id was not trivially copyable even before this change. It could be made such, if id::data permits (which it doesn't sometimes), but that would be for a different PR.

[pdimov]

The implicit members are only deprecated on C++11, so =defaulting them on C++11 doesn't raise a minimum requirement.

[pdimov]

Yes, id has a manually declared copy constructor. I don't know why this test used to pass.

[pdimov]

but that would be for a different PR.

I'm not sure about that; applying the =default change will automatically make it trivial.

[Lastique]

Ok, I can change the tests to use =default copy constructors conditionally. Although personally, I like the current code more as it is not conditional.

I'm hesitant about id because some (older) compilers deduce noexcept incorrectly for defaulted functions, and move assignment/constructor must be noexcept. I think, older gcc and Intel had this problem. We don't have a Boost.Config macro to detect these buggy compilers.

[Lastique]

Also, the move constructor/assignment are not default anyway, as they need to assign zero to the move source.

[pdimov]

There is no such requirement. It's normal and expected for trivial types to perform a copy on move. But the move operations of id, and the discussion about them, really belong in a separate PR.

[Lastique]

Ok, I've updated the PR:

• Changed the tests to use defaulted constructors conditionally.
• Removed move support from thread::id.
• Left thread::id non-trivially copyable because of the buggy compilers wrt. noexcept deduction.

Also, the test failure is because testable_mutex incorrectly uses atomic<thread::id>. Boost.Atomic was modified not long ago to issue a static assert that the type is trivially copyable. The requirement has always been there, it's just it was not enforced before.

testable_mutex needs to be fixed in a separate PR. Probably, by using the underlying thread handle type or thread_data* in the atomic.

[Lastique]

There is no such requirement. It's normal and expected for trivial types to perform a copy on move.

Yes, but id is not trivial. And when a smart pointer is used internally the move source is left in the default-constructed state (i.e. equal to id()). I think, the behavior should be consistent regardless of the internal representation of id.

[Lastique]

I've updated the PR:

• Rebased on current develop.
• Removed copy constructor of thread::id to allow all constructors/assignment to be generated, including move.

[Lastique]

Thanks, Peter.