Is CppCoreGuidelines C.21 correct?

While reading the Bjarne Stroustrup's CoreCppGuidelines, I have found a guideline which contradicts my experience.

The C.21 requires the following:

If you define or =delete any default operation, define or =delete them all

With the following reason:

The semantics of the special functions are closely related, so if one needs to be non-default, the odds are that others need modification too.

From my experience, the two most common situations of redefinition of default operations are the following:

#1: Definition of virtual destructor with default body to allow inheritance:

class C1
{
...
    virtual ~C1() = default;
}

#2: Definition of default constructor making some initialization of RAII-typed members:

class C2
{
public:
    int a; float b; std::string c; std::unique_ptr<int> x;

    C2() : a(0), b(1), c("2"), x(std::make_unique<int>(5))
    {}
}

All other situations were rare in my experience.

What do you think of these examples? Are they exceptions of the C.21 rule or it's better to define all default operations here? Are there any other frequent exceptions?

回答1:

I have significant reservations with this guideline. Even knowing that it is a guideline, and not a rule, I still have reservations.

Let's say you have a user-written class similar to std::complex<double>, or std::chrono::seconds. It is just a value type. It doesn't own any resources, it is meant to be simple. Let's say it has a non-special-member constructor.

class SimpleValue
{
    int value_;
public:
    explicit SimpleValue(int value);
};

Well, I also want SimpleValue to be default constructible, and I've inhibited the default constructor by providing another constructor, so I need to add that special member:

class SimpleValue
{
    int value_;
public:
    SimpleValue();
    explicit SimpleValue(int value);
};

I fear that people will memorize this guideline and reason: Well, since I've provided one special member, I should define or delete the rest, so here goes...

class SimpleValue
{
    int value_;
public:
    ~SimpleValue() = default;
    SimpleValue();
    SimpleValue(const SimpleValue&) = default;
    SimpleValue& operator=(const SimpleValue&) = default;

    explicit SimpleValue(int value);
};

Hmm... I don't need move members, but I need to mindlessly follow what the wise ones have told me, so I'll just delete those:

class SimpleValue
{
    int value_;
public:
    ~SimpleValue() = default;
    SimpleValue();
    SimpleValue(const SimpleValue&) = default;
    SimpleValue& operator=(const SimpleValue&) = default;
    SimpleValue(SimpleValue&&) = delete;
    SimpleValue& operator=(SimpleValue&&) = delete;

    explicit SimpleValue(int value);
};

I fear CoreCppGuidelines C.21 will lead to a ton of code that looks just like this. Why is that bad? A couple of reasons:

1. This is far more difficult to read than this correct version:

class SimpleValue
{
    int value_;
public:
    SimpleValue();
    explicit SimpleValue(int value);
};

2. It is broken. You'll find out the first time you try to return a SimpleValue from a function by value:

SimpleValue
make_SimpleValue(int i)
{
    // do some computations with i
    SimpleValue x{i};
    // do some more computations
    return x;
}

This won't compile. The error message will say something about accessing a deleted member of SimpleValue.

I've got some better guidelines:

1. Know when the compiler is defaulting or deleting special members for you, and what defaulted members will do.

This chart can help with that:

If this chart is far too complex, I understand. It is complex. But when it is explained to you a little bit at a time it is much easier to deal with. ~~I will hopefully be updating this answer within a week with a link to a video of me explaining this chart.~~ Here is the link to the explanation, after a longer delay than I would have liked (my apologies): https://www.youtube.com/watch?v=vLinb2fgkHk

2. Always define or delete a special member when the compiler's implicit action is not correct.

3. Don't depend on deprecated behavior (the red boxes in the chart above). If you declare any of the destructor, copy constructor, or copy assignment operator, then declare both the copy constructor and the copy assignment operator.

4. Never delete the move members. If you do, at best it will be redundant. At worst it will break your class (as in the SimpleValue example above). If you do delete the move members, and it is the redundant case, then you force your readers to constantly review your class to make sure it is not the broken case.

5. Give tender loving care to each of the 6 special members, even if the result is to let the compiler handle it for you (perhaps by inhibiting or deleting them implicitly).

6. Put your special members in a consistent order at the top of your class (only those you want to declare explicitly) so that your readers don't have to go searching for them. I've got my favorite order, if your preferred order is different, fine. My preferred order is that which I used in the SimpleValue example.

回答2:

I think maybe your second example is a reasonable exception, and, after all, the guideline does say "the odds are...", so there will be some exceptions.

I'm wondering if this slide might help with your first example:

Here are the slides: https://accu.org/content/conf2014/Howard_Hinnant_Accu_2014.pdf

EDIT: For further information regarding the first case, I've since discovered this: C++11 virtual destructors and auto generation of move special functions