16

I have a NonCopyable class and an Application class derived from NonCopyable:

class NonCopyable
{
public:
    NonCopyable() = default;
    virtual ~NonCopyable() = default;
    NonCopyable(NonCopyable const&) = delete;
    NonCopyable& operator =(NonCopyable const&) = delete;
    NonCopyable(NonCopyable&&) = delete;
    NonCopyable& operator=(NonCopyable&&) = delete;
};

class Application : public NonCopyable
{
public:
    ~Application() { /* ...delete stuff... */ }
};

As you see, I don't need to redeclare deleted move constructors or assignment operators as I've declared them in the base class. But why does Resharper suggest me to declare other special member functions? It said:

Class Application defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [hicpp-special-member-functions].

There're also [cppcoreguidelines-special-member-functions] reminders with the same message.

The current workaround is to explicitly write all of them to make the warning go away:

class Application : public NonCopyable
{
public:
    ~Application() { /* ...delete stuff... */ }
    Application(Application const&) = delete;
    Application& operator =(Application const&) = delete;
    Application(Application&&) = delete;
    Application& operator=(Application&&) = delete;
};
Improve this question
5
  • 5
    Please have a look at the rule of three / five / zero. Commented Jan 24, 2018 at 12:34
  • 1
    If you need to "delete stuff" in the destructor, you most often need to "copy stuff" in the copy constructor. Commented Jan 24, 2018 at 12:35
  • @BoPersson For this example, I don't need another Application instance (singleton) so there isn't a need of copy constructor. Commented Jan 24, 2018 at 12:38
  • Hi MiP, Resharper has its own logic based on which it create a list of warning. If warning are not fatal you can just ignore it. It not necessary that you have done some mistake. Commented Jan 24, 2018 at 12:42
  • 1
    This particular inspection is coming from clang-tidy, not from ReSharper itself (it just invokes clang-tidy automatically on your source code). You can distinguish clang-tidy inspections by their name in square brackets at the end of the message. You can also quickly consult inspection documentation by pressing Alt+Enter on the inspection, then going into inspection submenu and selecting Open documentation. In the same submenu you can change inspection severity or switch it off altogether. Commented Jan 24, 2018 at 19:19

3 Answers 3

Reset to default
6

More often than not, if a class defines a destructor that differs from what the compiler would automatically generate, its purpose is to release some resource that is managed by the class.

If there is some resource that a destructor must release then there is usually also a need for a user-defined constructor to initialise that resource, a user-defined copy constructor to create a copy of the resource owned by an existing object of the same type, and a user-defined move constructor to assume ownership of the resource from a temporary object that is ceasing to exist.

Similarly, there will also be a need for an copy assignment operator and move assignment operator, so that expressions like a = b will work to copy or move the resource to an existing objects.

If the class does not have one or more of these constructors or assignment operators then, practically, code which uses several objects often will not behave consistently (e.g. a destructor releasing a resource twice because it is shared by two objects, resulting in undefined behaviour, etc).

Of course, there are circumstances in which a class does not need the complete set of constructors, assignment operators, and destructor. However, leaving one of them out is often a programmer error (as distinct from an intended effect) so code analysis tools will often complain about such cases.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

5

default, delete are not inherited

You are wrongfully assuming some kind of transitivity (When constructing an object, I always need to use the superclass constructor with a similar signature).

Here is a counter-example :

#include <string>
#include <iostream>

class NonCopyable
{
public:
    NonCopyable() = default; // Added this little fella
    virtual ~NonCopyable() = default;
    NonCopyable(const NonCopyable &) = delete;
    NonCopyable& operator =(NonCopyable const&) = delete;
    NonCopyable(NonCopyable&&) = delete;
    NonCopyable& operator=(NonCopyable&&) = delete;
};

class Application : public NonCopyable
{
public:
    Application() = default;
    Application(const Application& a) : NonCopyable(), x(a.x){}
    ~Application() { /* ...delete stuff... */ }
    std::string x;
};

int main(){
   Application a;
   a.x = "Hello";
   Application b(a);

   std::cout << b.x << std::endl;

}

As you can see I am perfectly able to declare copy constructor of a subclass by using the only available constructor of the superclass. I could also declare the other constructors of the rule of 5. This is why static analyzers are giving warning about the rule of 5 .

Conclusion : NonCopyable is a useless class.

Improve this answer

4 Comments

When editing the post, I mistakenly delete the constructor.
I'm sorry, but I don't get the counter-example. What I want is prevent me from doing Application a; auto b = new Application(a); and only declare copy constructor when needed. The NonCopyable class helps me do this.
I updated the counter example to make the call to the copy constructor more explicit. see ideone.com/Rr5h0U
"As you can see I am perfectly able to declare copy constructor of a subclass by using the only available constructor of the superclass.": this is what I want. NonCopyable makes me explicitly declare Application(const Application& a) in the subclass, instead of being automatically and implicitly created. And when I need to manually write the copy constructor, it then suggests me to conform the rule of 5.
1

For all ReSharper knows, you defined a non-trivial destructor and did not follow up with the rest of the rule of 3/5. Yes, the copy/move constructors are deleted by default since they are deleted in the base class, but how would ReSharper know this is desirable behavior? Therfore it makes sense for it to warn you unless you explicitly mark them as delete.

You can just disable the warning locally with a pragma or delete the constructors/assignments explicitly as you suggested.

As a workaround you can introduce a virtual cleanup() method, call it from the NonCopyable destructor, and override it (instead of the destructor) in the derived classes.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.