About sonar cloud C++11 constructor rule Is it BUG?

sonarcloud
sonarcfamily

(Freeeyes) #1
  • version used: SonarCloud
  • error observed (wrap logs/code around triple quote ``` for proper formatting)
    My project uses sonar check tips:
    Explicitly define the missing copy constructor, move constructor, copy assignment operator and move assignment operator so that they will not be implicitly provided.
  • steps to reproduce
    I think constructor, copy constructor, move constructor, copy assignment operator and move assignment operator,destruct
    copy constructor, move constructor, copy assignment operator and move assignment operator is not necessary, They can be constructed by default. Do they have to be implemented? or it’s a bug?
    I know the C++ 11 standard may use this. Does each class implement these functions?
    so, please Reduce the error level?
    Thanks.

and then add function tell me:

Link to SonarCloud issue:
https://sonarcloud.io/project/issues?id=freeeyes&open=AWkyOnwxSHkPyAs1DKKz&resolved=false&types=BUG


(Loïc Joly) #4

Hello @freeeyes,

Indeed, we added two related rules:

  • One is related to the rule of zero, and is a code smell, tells not to write any special member function, but let the compiler define them for you
  • The second, which is related to the rule of five only applies when you are in a situation where the rule of zero is not applicable. It tells you that if you define one special member function, but not all five, the code will certainly have some issue.

We believe those rules can still be fine-tuned, but in your case, I think they reveal some interesting issues in your code… Let’s take one of your example:

The class CMessageanager defines a destructor, but no other special member function. So we raise both rules. In which direction should you go? (0 special member functions, or 5 member functions?). By default, it should be 0 special member function. And here, since the destructor contains only log, removing it could be an option, if you can accept not to have those log messages. If you really need those logs, there are still several options, and one of the would be to define all 5 special member functions.

Why do we qualify the rule of 5 as a bug? In this case, for example, it is a performance bug. Because you manually defined a destructor, and nothing else, the compiler will generate the following:

  • a copy constructor, that copies all member data
  • a copy assignment operator, that assigns all member data

If the class did not have a destructor, this is what the compiler would have generated:

  • a destructor
  • a copy constructor, that copies all member data
  • a copy assignment operator, that assigns all member data
  • a move constructor, that moves all member data
  • a move assignment operator, that assigns all member data by moving them

The consequence of adding the destructor (and not the 4 other special member functions), is that the class is now more expensive to move (because is will have to use the copy constructor, even in situations where the move constructor could have been selected if it existed…).

I hope this clarifies the situation.


(Freeeyes) #5

I got it, thinks. @JolyLoic