40

In order to make an object non-copiable we can explicitly delete both its copy-constructor and copy-assignment operator.

My question is: What is the right place to do it - in the public, private or protected section of the class? And - does this choice make any difference?

StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
Sajal
  • 1,783
  • 1
  • 17
  • 21

5 Answers5

46

what is the right place to do it - in the public, private or protected section of the class?

I would put them in the public section.

This is because deleting a constructor or an assignment operator is orthogonal to making them private / protected; and when these aren't deleted, they are public by default. Putting the deletions in one of those two sections seems to me like hinting "If I hadn't deleted them, I would have made them private/protected" - which is not a message you want to convey in your case.

Note, though, that the compiler doesn't care which section you put the deletion in.

einpoklum
  • 118,144
  • 57
  • 340
  • 684
  • 10
    Exactly this. Back in the day we made these things private specifically to deny folks access to them, but _this was always a hack_ and was **only** because we couldn't `delete` them. That consideration is no longer in play. I don't remember whether the "copy constructor is private" diagnostic tends to take precedence over the "copy constructor is deleted" diagnostic (I doubt it) but even if it doesn't changing the access level is not the right thing to do for the reasons you give. – Lightness Races in Orbit Mar 17 '19 at 16:32
  • @LightnessRacesinOrbit I know I've seen some compilers give both errors when a function is private and deleted. The one about private access becomes just extra noise. – aschepler Mar 17 '19 at 21:55
  • @aschepler Fair does – Lightness Races in Orbit Mar 17 '19 at 22:03
  • gcc 7.4, but not gcc 8.1: https://godbolt.org/z/udzwB2 (so I guess they improved that). – aschepler Mar 17 '19 at 22:24
  • @aschepler: Hmm... you switched the output tabs in there, so it almost looks like it's 7.4 that's doing the right thing. Anyway, thanks. – einpoklum Mar 17 '19 at 22:37
  • Huh. I think they were arranged correctly when I clicked "Share". Oh well. – aschepler Mar 17 '19 at 22:40
21

Does where we put the deleted definition make any difference?

From a pure language standpoint it makes absolutely zero difference. Name lookup and overload resolution happen before access checking. And attempting to refer to a deleted function at the end of overload resolution makes your program ill-formed, period. A compiler may or may not issue another diagnostic about the accessibility, but the program already has an error that must be reported.

So you can put that deleted definition with whatever accessibility you desire. I think most will keep it private, to be inline with the "old" practice of making a class non-copyable (put the declaration of those members in the private section of the class, and not define them), if only to help those who know the old ways "get it" sooner. A mixture of idioms, if you would.

Marking as private is also something you can't avoid if you need to support both C++03 and C++11 mode. With the help of a macro, a header can be made to conform to both standards easily:

#if __cplusplus >= 201103L
  #define DELETED_DEFINITION = delete
#else
  #define DELETED_DEFINITION
#endif

class noncopyable {
private:
  // This header can be compiled as both C++11 and C++03
  noncopyable(noncopyable const&) DELETED_DEFINITION;
  void operator=(noncopyable const&) DELETED_DEFINITION;
};
StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
  • _If_ you want backwards compatibility then this is a must. – Lightness Races in Orbit Mar 17 '19 at 16:32
  • 2
    @LightnessRacesinOrbit If you want backward compability, you should NEVER use C++11 features. In the case of deleted functions, the workaround is proposed by StoryTeller. But what is the backward compability solution when using lambdas, stl, concurrency, etc. ? – TonySalimi Mar 18 '19 at 08:23
  • 2
    @hsalimi There's only so far you can go with C++11 features if you want a useful compatibility layer, true, but I did have an event framework lib in a previous project that could be compiled in either C++03 or C++11 mode (it was still used by legacy embedded projects), and in the latter case it had a bunch of optimisations (mostly relating to rvalue refs) that improved things greatly without bunging up the interface too badly. I did need it to switch between Boost.Thread and `std::thread`, but then in the latter case it alleviates a library (and link!) dependency so it's not for naught. – Lightness Races in Orbit Mar 18 '19 at 11:23
  • @hsalimi But yeah it meant I couldn't use lambdas in the lib ... or, at least, doing so would have been more trouble than it were worth. – Lightness Races in Orbit Mar 18 '19 at 11:24
17

From Scott Meyers's book, Effective Modern C++ (Item 10), it seems that it is better to define them as public:

By convention, deleted functions are declared public, not private. There’s a reason for that. When client code tries to use a member function, C++ checks accessibility before deleted status. When client code tries to use a deleted private function, some compilers complain only about the function being private, even though the function’s accessibility doesn’t really affect whether it can be used. It’s worth bearing this in mind when revising legacy code to replace private-and-not-defined member functions with deleted ones, because making the new functions public will generally result in better error messages.

In addition, I believe that a deleted copy constructor/assignment, should be part of the class interface to be shared with ALL of the class users. Such kind of information should not be kept as secret by making them private.

TonySalimi
  • 8,257
  • 4
  • 33
  • 62
  • 3
    Meyers contradicts StoryTeller's answer, and [my tests](http://coliru.stacked-crooked.com/a/9515319644f041db). It's still good advice, but I find einpoklum's reasoning to be superior. – Lightness Races in Orbit Mar 17 '19 at 22:04
  • @LightnessRacesinOrbit I did the check on VS2013. The error message is different than g++ and it rightly shows the error. In addition, from a conceptual point of view, when a class deletes a ctor/cctor, etc., the class wants to say to ALL of its clients that: "Hey, this member is deleted, and you are not allowed to use it". There is no secret about this fact to keep it private. – TonySalimi Mar 18 '19 at 08:18
  • Agreed; that is einpoklum's reasoning that I find excellent ;) – Lightness Races in Orbit Mar 18 '19 at 11:25
7

delete works just as well with private access.

The effect of delete is to cause an error if the function is chosen by overload resolution.

The effect of private is to cause an error if the function is chosen by overload resolution from outside the class or its friends.

If both errors apply, the ultimate outcome is the same either way, but public might help avoid compiler messages about access privileges, which could cause confusion.

Potatoswatter
  • 134,909
  • 25
  • 265
  • 421
  • 1
    It is recomended to make deleted functions public. See https://stackoverflow.com/a/18931192/108238. Also Clang-Tidy suggests this. – schoetbi Feb 06 '18 at 12:03
2

The access of a deleted function is irrelevant. In fact, for class members, it would have made more sense to add an additional access specifier (delete:). I suspect the reason they didn't do that, was that it wouldn't work for non-member functions.

For things like the copy constructor, it makes more sense stylistically to put it in the public section. The fact that a class doesn't have a copy constructor is a pretty major fact to know about the interface to the class.

For internal functions where you are declaring a particular overload as deleted in order to get compiler-time detection of an error, it makes sense to declare the function in the same section as all the other overloads.

Martin Bonner supports Monica
  • 28,528
  • 3
  • 51
  • 88