I'm getting the following message from Clang-Tidy:
Clang-Tidy: Calling a base constructor other than the copy constructor
The code is this:
#include <memory>
#include <set>
using namespace std;
class Node : enable_shared_from_this<Node>
{
public:
explicit Node(Node *parent = nullptr) : parent(parent) { parent->children.insert(shared_from_this()); }
Node(const Node &that) : parent(that.parent), children(that.children) {}
private:
Node *parent; // C 14 "weak pointer"
set<shared_ptr<Node>> children;
};
I can't find documentation for this message from Clang-Tidy anywhere.
What does it mean and why is it giving it?
CodePudding user response:
Clang-Tidy is trying to warn you that your copy constructor fails to call copy constructor of the base class. GCC has similar warning under -Wextra
:
<source>: In copy constructor 'Node::Node(const Node&)':
<source>:10:5: warning: base class 'class std::enable_shared_from_this<Node>' should be explicitly initialized in the copy constructor [-Wextra]
10 | Node(const Node &that) : parent(that.parent) { if (that.parent) that.parent->children.insert(shared_from_this()); }
| ^~~~
You should explicitly use copy constructor in member initializer list:
Node(const Node &that) : enable_shared_from_this(that), parent(that.parent) { if (that.parent) that.parent->children.insert(shared_from_this()); }
There are however other issues with your use of enable_shared_from_this
. First, you must always inherit that class publicly:
class Node : public std::enable_shared_from_this<Node>
// ~~~~~~~~~~^
Also, you are not allowed to call shared_from_this()
when the owning std::shared_ptr
is not yet constructed (notably in class constructor). In C 17 this doing so will throw std::bad_weak_ptr
, in earlier standard versions this is Undefined Behaviour.
CodePudding user response:
The message is relatively quite clear in my opinion, assuming it points to the copy constructor specifically. It could be improved by referring to the problematic base as well though.
Node
inherits enable_shared_from_this<Node>
. The copy constructor which you define manually does not mention this base class in the initializer list. Therefore the base class will be default-initialized. Having a base class or member be default initialized in a copy constructor is very likely to not have the intended effect and much more likely to be an oversight. This is why clang-tidy produces the diagnostic.
In this particular situation here there is no problem with enable_shared_from_this<Node>
being default-initialized, since its copy constructor is defined to perform the same action as its default constructor. But that is a rare exception.
You can satisfy clang-tidy and avoid relying on this special case, by correctly initializing the base as well:
Node(const Node &that) : enable_shared_from_this(that), parent(that.parent) { /*...*/}
However, you have more serious problems here. First, std::enable_shared_from_this
must be inherited publicly. You are currently inheriting it privately.
Second, you cannot use shared_from_this
inside a constructor. When the constructor runs, the object which is being constructed is not yet under control of a std::shared_ptr
. As a result calling shared_from_this
will simply throw std::bad_weak_ptr
(since C 17; undefined behavior before that).