NFD » ndn-cxx

boost::noncopyable should not be required in every case (it makes implementing move semantics much harder). With C++11 we can simply delete the copy constructor and copy assignment operator.

Alternatively we could have a movable_but_not_copyable (find a shorter name) base type:

class movable_but_not_copyable
{
public:
  movable_but_not_copyable() = default;
  movable_but_not_copyable(const movable_but_not_copyable&) = delete;
  movable_but_not_copyable(movable_but_not_copyable&&) = default;

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

  movable_but_not_copyable&
  operator=(movable_but_not_copyable&&) = default;

I don't understand why we so paranoid with this copyable/noncopyable. This make sense for classes that actually used in places that could be copied, but doesn't make sense for many other cases (especially one-time use classes).

Use of noncopyable and deleted constructors should be made on case-by-case basis and not forced throughout the code.

Use of noncopyable and deleted constructors should be made on case-by-case basis and not forced throughout the code.

Developer usually won't think about whether the type should be movable/copyable, and sometimes end up copying an expensive structure.

By forcing either noncopyable or a justification, developer is making an informed choice about this question.

Some common justifications should be listed along with this rule, such as:

• This type is copyable so that it can be stored in STL containers.
• This type is copyable because it represents a TLV structure.

This should be dealt when there is a problem. Many cases where you forced us to use boost:noncopyable, such thing simply doesn't make sense, as it is either one-time use class or it is never used in copy operations (and in some cases it simply doesn't have any variables to copy).

For the thing that "sometimes" and "maybe" expensive, forcing everything to conform to some standardized way is not correct. I'm objecting here again to introduce any rule. When it make sense, I'm very positive in having noncopyable or removed constructors/copy operators. HOwever, this needs to be added where necessary, decided on case-by-case basis.

The main purpose of this rule is to provide guidelines to developers, and to remind developers to think about whether each type should be copyable or noncopyable.

noncopyable or copyable justification illustrates an informed choice has been made on a type.

Many cases where you forced us to use boost:noncopyable, such thing simply doesn't make sense, as it is either one-time use class or it is never used in copy operations (and in some cases it simply doesn't have any variables to copy).

This rule can specifically list a number of situations where neither noncopyable nor copyable justification is needed.

Or more preferably, give a standard list of copyable justifications which can be copy-n-pasted into the code.

For the thing that "sometimes" and "maybe" expensive, forcing everything to conform to some standardized way is not correct. I'm objecting here again to introduce any rule. When it make sense, I'm very positive in having noncopyable or removed constructors/copy operators. However, this needs to be added where necessary, decided on case-by-case basis.

This (and any other) rule never forbids exceptions. To request an exception, copy-n-paste a standard copyable justification into the code. Exception is granted when code review passes.

Alex Afanasyev wrote:

This should be dealt when there is a problem. Many cases where you forced us to use boost:noncopyable, such thing simply doesn't make sense, as it is either one-time use class or it is never used in copy operations (and in some cases it simply doesn't have any variables to copy).

We already had at least one problem. I forgot where, but in that case noncopyable was preventing the compiler from generating a trivial default move constructor for a parent class, thus making a derived type non movable by default, even after defaulting the move constructor in the derived class.

This was completely non-obvious, and clearly shows that there's a problem with blindly forcing noncopyable everywhere like Junxiao has been doing lately. So I actually agree with Alex here that we should NOT insist on making classes noncopyable "by default".

I agree with note-6 that boost::noncopyable isn't suitable because it conflicts with C++11 move semantics.

However, I still want to ensure developers make an informed decision on whether each type is movable / copyable, and such decisions should be reflected in code or comment.

How can this be achieved?

One of the early uses of noncopyable is #1252.

One possible solution is to require either of:

• noncopyable
• movable_but_not_copyable + a test case for move semantics
• copyable justification + a test case for copy semantics

My position on this issue remains that no additional requirements should be universally imposed. The need to delete default copy constructor/copy operator/use of noncopyable base should be made on case by case basis. Some classes are inherently used only once (like runners in main.cpp's) and it is unnecessary adding any extra restrictions.

The decision what to do in each case should be informed and we can ask developers such thing during the code review. However, I see no need to require putting anything in comments for the class.

The decision what to do in each case should be informed and we can ask developers such thing during the code review. However, I see no need to require putting anything in comments for the class.

How could the decision and the reason behind it be documented?

My point is that in many cases it does not need to be documented at all.

My point is that in many cases it does not need to be documented at all.

No. Every decision needs to be documented. Otherwise, in the future,

• Someone may ask "why this type is copyable?"
• It's not always easy to come up with an answer.
• Changing to noncopyable at that time may unexpectedly break things. (eg. a type is used in STL container outside of ndn-cxx)