23

The following surely works but is very tedious:

T(const T&) = delete;
T(T&&) = delete;
T& operator=(const T&) = delete;
T& operator=(T&&) = delete;

I'm trying to discover the most concise way. Will the following work?

T& operator=(T) = delete;

Update

Note that I choose T& operator=(T) instead of T& operator=(const T&) or T& operator=(T&&), because it can serve both purposes.

Baum mit Augen
  • 49,044
  • 25
  • 144
  • 182
Lingxi
  • 14,579
  • 2
  • 37
  • 93
  • 4
    No. Use the 4-lines version to explicitly disable all 4. – iBug Feb 19 '18 at 11:19
  • 2
    what about base class? – Yola Feb 19 '18 at 11:23
  • It seems in the standard preferably copy constructor and copy assignment operator are deleted. Compare [this discussion](https://stackoverflow.com/q/27780647/3876684). – Claas Bontus Feb 20 '18 at 08:08
  • `boost::noncopyable` is a concise and expressive approach, if you don't mind paying for the dependency (or already have it) – ricab Feb 20 '18 at 18:06
  • @ricab I have been somewhat sick of Boost since C++11, because its commitment of back compatibility with C++03. – Lingxi Feb 21 '18 at 02:22

6 Answers6

41

According to this chart (by Howard Hinnant):

meow

The most concise way is to =delete move assignment operator (or move constructor, but it can cause problems mentioned in comments).

Though, in my opinion the most readable way is to =delete both copy constructor and copy assignment operator.

Smi
  • 13,850
  • 9
  • 56
  • 64
HolyBlackCat
  • 78,603
  • 9
  • 131
  • 207
4

You can write a simple struct and inherit from it:

struct crippled
{
    crippled() = default;

    crippled(const crippled&) = delete;
    crippled(crippled&&) = delete;

    crippled& operator=(const crippled&) = delete;
    crippled& operator=(crippled&&) = delete;
};

Usage:

struct my_class : crippled
{

};

int main()
{
    my_class a;
    auto b = a; // fails to compile
}
Vittorio Romeo
  • 90,666
  • 33
  • 258
  • 416
  • 2
    I believe Boost has a class like this. It would be more transparent to inherit from that instead of rolling your own (although not worth using Boost in your project if that's the *only* use). – Arthur Tacca Feb 19 '18 at 14:37
  • 7
    That would be boost::noncopyable (http://www.boost.org/doc/libs/1_65_0/libs/core/doc/html/core/noncopyable.html) – Pablo Oliva Feb 19 '18 at 15:44
4

I prefer to inherit from boost::noncopyable, thereby making the intention immediately clear and delegating the details to a trustworthy library.

#include <boost/core/noncopyable.hpp>

class X: private boost::noncopyable
{
};

It involves adding a dependency, but if you are ok with that, it is arguably a very concise and expressive way to accomplish it.

ricab
  • 2,697
  • 4
  • 23
  • 28
4

Please don't try to find "the most concise way" to write a piece of code.

If there isn't an obvious form of expressing something which is also very concise - don't try to language-lawyer your way into writing a few characters less. Why? Think of people reading your code: If you need to consult the standard to realize your code does what you want it to do - then so will your code's readers. Except that they won't know what you're trying to achieve; so they won't consult the standard; so they'll just be confused about what your code does. Or - some will get it and some wont.*

In your case, if you make a subset of these deletions, or use some other "clever" trick - as a person reading your code, it's somewhat likely I will not catch on, not notice you're actually trying to get all copy and move semantics deleted. And I'll get confused, thinking you're trying to do something else. In fact, if I were you, I'd even consider adding a comment saying:

/* Disabling copy and move semantics because XYZ */
T(const T&) = delete;
T(T&&) = delete;
T& operator=(const T&) = delete;
T& operator=(T&&) = delete;

which is even more "tedious", but would make your intention/motivation absolutely clear to your future readers.

There's also the question of what exactly the "XYZ" reason is. Some would argue that there's no good reason to delete the move members, and it's generally a bad idea to do so. C++ luminary Howard Hinnant has this to say on the matter.

* - A variant on a principle I spelled out here.

einpoklum
  • 118,144
  • 57
  • 340
  • 684
  • 1
    You are assuming the most concise way is less expressive, which is not necessarily the case. The intention is clearer if it can be expressed concisely while maintaining precision. – ricab Feb 20 '18 at 17:29
  • @ricab: See edit. The point is that "most concise" is not a virtue. – einpoklum Feb 20 '18 at 17:31
  • 2
    I maintain that it is, if it does not go against expressiveness. `boost::noncopyable` is a good example off achieving both IMO – ricab Feb 20 '18 at 17:33
  • @ricab but it isn't. I still had to doublecheck whether `boost::noncopyable` disables only copying or if it disables moving too. Moving a value is NOT copying. – Kaihaku Sep 15 '21 at 15:18
  • @Kaihaku, you need to know what the building blocks used in the code do. That is always the case and better than rewrite the code each time. Going for `noncopyable` is essentially just a case of [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself). The name could probably be better, but you know, history... – ricab Sep 15 '21 at 17:57
  • @ricab: Your exchange with Kaihaku just gave me another idea. – einpoklum Sep 15 '21 at 19:51
3

I believe that in this case, macros are actually more readable:

#define NOT_COPYABLE( TypeName ) \
TypeName ( TypeName const& ) = delete; \
TypeName & operator = ( TypeName const& ) = delete;

#define NOT_MOVEABLE( TypeName ) \
TypeName ( TypeName && ) = delete; \
TypeName & operator = ( TypeName && ) = delete;
KevinZ
  • 3,036
  • 1
  • 18
  • 26
1

@ricab's answer suggested not inventing the wheel, and using boost::noncopyable as a mix-in base class. That does indeed work, but has the disadvantage of being confusing! ... as being non-copyable does not mean a class is non-movable. One would have to remember the historical circumstances for the naming of this boost class; most people sure don't, and neither do I.

Well, you could do the same, but renaming this class:

#include <boost/core/noncopyable.hpp>

namespace mixins {
using noncopyable_and_nonmovable = boost::noncopyable;
} // namespace mixins 

class X: private mixins::noncopyable_and_nonmovable {
  // etc. etc.
};

Now what you're doing is clear and obvious :-)

Also, you may want to consider creating a small namespace of mixins with this definition and possibly others (e.g. only non-copyable, only-non-movable etc.) into some utility header. That would make this solution even more concise to use.


I would still consider explaining the reason for the decision to disallow copying and moving somewhere; see my other answer.

einpoklum
  • 118,144
  • 57
  • 340
  • 684
  • OK yeah, that's probably a good idea. I don't know about "most people" but surely "some people", and that's enough if this helps them. Especially since docs for `boost::noncopyable` are silent with respect to move. It follows from language rules that they are deleted (see most voted answer), but may as well make it clear in the name. – ricab Sep 16 '21 at 10:49
  • @ricab: It's a pretty safe assumption that most people using a library don't know the history of its development; and an ever safer assumption about a large library with a long history like Boost. I would absolutely not have guessed that `boost::noncopyable` is non-movable and I consider myself not completely clueless. – einpoklum Sep 16 '21 at 18:18
  • History may explain the name, but the docs for noncopyable say copy operations are deleted. It follows that no move operations are generated. I am all for names being the first source of documentation, but they are still not the *only* one. Anyway, you already got my upvote. – ricab Sep 16 '21 at 19:37
  • @ricab: Well, unique pointers have copy ctors deleted, with move operators not deleted. So it's not an obvious assumption to make, that non-copyable class is non-movable. And we can't rely on people scrutinizing the mixin class' source. – einpoklum Sep 16 '21 at 21:18