Thursday, March 13, 2014

A Concern about the Rule of Zero

The Rule of Zero, coined, as far as I know, by R. Martinho Fernandes in this blog post, is:

Classes that have custom destructors, copy/move constructors or copy/move assignment operators should deal exclusively with ownership. Other classes should not have custom destructors, copy/move constructors or copy/move assignment operators.

By "not have," Martinho really means "not declare", because all classes have destructors, one way or another, and the blog post makes clear that Martinho expects a non-resource-handling class to have the copying and moving behavior exhibited by its data members (and hence to have the corresponding functions, assuming such functionality is employed). If we revise the wording of the rule in accord with s/have/declare/g, we get:

In concept, it's great. Morally, I'm on board. I agree that classes that don't manage resources should be designed so that the compiler-generated functions for copying, moving, and destruction do the right things. I'm just not sure that taking advantage of that by not declaring any of these functions is a good idea.

Consider a class Widget that doesn't do resource management. Per the Rule of Zero, it declares none of the five special functions covered by the rule. Further assume that its data members are both copyable and movable. Widget objects are therefore copyable and movable, too.

class Widget {
public:
... // no dtor or copy or move functions
};

Wonderful. Life is good.

Now assume something in the software doesn't work the way it should. It could be a behavioral problem (i.e., your run-of-the-mill bug) or it could be a performance problem. Either way, debugging ensues. Let's assume that during debugging, it becomes convenient to temporarily add a destructor to do something like produce a log message for tracing purposes:

The addition of the destructor has the side effect of disabling generation of the move functions, but because Widget is copyable, all the code that used to generate moves will now generate copies. In other words, adding a destructor to the class has caused presumably-efficient moves to be silently replaced with presumably-less-efficient copies. That strikes me as the kind of thing that (1) is likely to surprise people and (2) could really complicate debugging. Hence my discomfort.

I'm inclined to recommend that a better way to rely on the compiler-generated copy and move functions is to expressly say that they do the right thing--to define them via =default:

With this approach, the spirit of the Rule of Zero remains:
classes that don't manage resources should be designed so that the
compiler-generated functions for copying, moving, and destruction do the
right things. But instead of expressing this by not declaring those functions, it's expressed by declaring them explicitly and equally explicitly opting in to the compiler-generated implementations.

What do you think? Does this advice make sense? Should the Rule of Zero perhaps be the Rule of the Five defaults?

25 comments:

First, I would want to have a "chat" with anyone in my team that willy-nilly adds destructors to classes and doesn't think about the Ro3 ;)

Then there's the fact that the generation of copies when there is a destructor present is now deprecated since C++11. Clang already warns about this behaviour, and GCC has an outstanding issue about it (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58407). Hopefully compiler writers will catch up on this.

If debugging ensues, why would you ever want to look at the Widget destructor? It was compiler generated after all. As long as all data ownership is delegated to proper classes, you should only have to look at those data-owning class destructors.

@Martinho Fernandes: Regarding feature deprecation, I think it's important to recognize that the committee is extremely conservative about actually getting rid of deprecated features. C++98 deprecated operator++ on bools, for example, but such code remains legal (albeit deprecated) in C++14. Applying operator++ to bools with gcc 4.8.1 with -Wall yields no warning, and the most recent CTP compiler from Microsoft has the same result with -W4. (I didn't check Clang.) I believe it's unwise to assume that because something is deprecated, it won't be used, especially if such use is accidental (as might well be the case when using the deprecated generation of copy operations for a class with a user-declared destructor).

In multithreaded code, the destructor is often an interesting case to study. When a bug occurs, it's a nice «breakpoint» spot to have. Having gone through some tricky cases moving fron C++03 to C++11 code for my PhD, I'm inclined to side with Scott on this one. Even though the bugs I've had were rarely due to the default destructor code, the side effects that stemmed from that method were quite a few times interesting to check in a debugger for analysis purposes.

If you can explicitly mark a function to be default then you should. Then changes to the class later have zero effect on the defaulted functions. No matter what the change is. Which is a good guarantee to have.

Strongly in favor of fixing this concern in the compiler, rather than confuse every class declaration with 5 default empty functions. Assume the people reading your C++ code are capable C++ programmers.

It's not really a new rule, the rule of 5 defaults, is it? At least not in this case. While the destructor in the example as shown is empty, the implication is that it will actually hold some code (logging). In that case wouldn't the "normal" rule of 5 apply? You've explicitly defined one, so you should define all 5, even if you are just declaring the other 4 as default.

I understand the original post was about whether the rule of 5 applies to non-resource controlling classes. In regards to that, I tend to agree with Scott that it should be more guidance than rule. But in the counter-example described, I don't think we are defining a new rule. It's just a very simple case of the existing rule.

@Karsten said:If you need tracing you can easily add a tracer class to your class members.

The downside of that is that it (likely, unless you can slip it into an existing padding region) changes the footprint of your class. That might also perturb performance measurements, although admittedly less that disabling moves would, likely.

FWIW, as I was reading this piece I was mentally asking myself, why doesn't Scott recommend that we explicitly qualify these implicit functions as default/delete as appropriate? As I continued reading I was happy to see that is what you eventually recommended.

I think the explicitly defaulted versions signal that the implementer was aware of them and explicitly wanted to state that they are fine. So for complex classes I think it might be a good idea to add them. OTOH if you just have a struct with a few public members and no fancy methods, they are likely more distracting then helping.

I think the advice on using =default for these are great.It gives clear documentation to the reader of the code that you want to have the compiler generated versions of these constructors and operators.

what i think people are missing here is the fact that afaik no ide/compiler gives you a nice feeling for what functions are compiler generated and also to breakpoint into them. tbh i find it kind of dumb that if I have a class that has member vector I cant put a breakpoint that will be fired when that vector starts to be destructed.but tbh luckily IDK if I ever needed that feature or to know what funciton are compiler generated (except ctor, dtor, asignment) so it is not that bad, but then again Im not writing high q code tbh.

@Mike G., @Karsten: You could as well create a small "tracer" class with a destructor, and temporary add it into an inheritance list of your "widget" class. If this tracer class has no members, it will not change memory layout of the widget. If you need to pass a name of your widget into the tracer's log, you could make the tracer a template with a char* as a template parameter (or pass the widget type itself and use RTTI.

However, my arguments slightly differ from Martinhos. I tell my students and listeners to write their classes in a way that the compiler-provided destructor, copy/move ctor/assignment automatically are correct (either defaulted or deleted).

All resource management should be put in the hand of library classes and corresponding members. new/delete are verboten (with the exception of initializing a unique_ptr until C++14). Polymorphic types that need to be kept on the heap should always be instantiated via make_shared(). This avoids the need to declare a virtual dtor, but only works if applied consistently (and doesn't work with unique_ptr).

To fill the gap that unque_ptr must be bend hard to fill (i.e., care for resources that aren't pointers), I promote a library addition for another resource Management classes in N3949 (scope_guard, unique_resource).

The argument of debugging is IMHO a weak one, and a deficit of the tooling. Code that doesn't need to be written usually can not be written wrongly: "less code = more software" and by my calling, developers shouldn't debug interactively, but should write good unit tests to avoid the need for debugging (I know, that is another hard to learn and follow topic).

To sum up: even for the average C++ developer the "rule of zero" is much easier to follow than anything else, because "things just work". Just don't use plain pointers, or self-written resource management classes. Rely on library experts to provide such for you.

I don't agree with the "rule of five defaults". In short, the advice reads like adding boilerplate-Big5 to any class that does not need them otherwise, just because under certain circumstances someone some day might want to add one of them and could forget to add the others.

I concur with Martinho that if anyone writes a destructor, constructor or op= for whatever reason, it should be a reflex to at least consider the others.

Maybe a compromise would suit, a "Rule of All or Nothing": As long as you can, stick to the Rule of Zero, but if you have to write at least one of the Big Five, default the rest.