Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

refactor: use shared_ptr to replace raw pointer #754

Merged
merged 5 commits into from
Feb 20, 2021

Conversation

zhangyifan27
Copy link
Contributor

No description provided.

@zhangyifan27 zhangyifan27 reopened this Feb 6, 2021
acelyc111
acelyc111 previously approved these changes Feb 7, 2021
src/replica/replica_stub.cpp Outdated Show resolved Hide resolved
src/replica/mutation_log.h Outdated Show resolved Hide resolved
@@ -49,7 +49,7 @@ class slave_failure_detector_with_multimaster : public dsn::fd::failure_detector
slave_failure_detector_with_multimaster(std::vector<::dsn::rpc_address> &meta_servers,
std::function<void()> &&master_disconnected_callback,
std::function<void()> &&master_connected_callback);
~slave_failure_detector_with_multimaster(void);
virtual ~slave_failure_detector_with_multimaster() {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
virtual ~slave_failure_detector_with_multimaster() {}
virtual ~slave_failure_detector_with_multimaster() override {}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neverchanje why need an override when it's already used virtual ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

override and virtual are for different purposes. virtual disclaims that this function might be inherited by subclasses. But override indicates that it overrides the parent, i.e dsn::fd::failure_detector.

Copy link
Contributor Author

@zhangyifan27 zhangyifan27 Feb 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://en.cppreference.com/w/cpp/language/virtual: Even though destructors are not inherited, if a base class declares its destructor virtual, the derived destructor always overrides it.

https://en.cppreference.com/w/cpp/language/override: In a member function declaration or definition, override specifier ensures that the function is virtual and is overriding a virtual function from a base class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly annotate overrides of virtual functions or virtual destructors with exactly one of an override or (less frequently) final specifier. Do not use virtual when declaring an override. Rationale: A function or destructor marked override or final that is not an override of a base class virtual function will not compile, and this helps catch common errors. The specifiers serve as documentation; if no specifier is present, the reader has to check all ancestors of the class in question to determine if the function or destructor is virtual or not.

levy5307
levy5307 previously approved these changes Feb 18, 2021
@@ -49,7 +49,7 @@ class slave_failure_detector_with_multimaster : public dsn::fd::failure_detector
slave_failure_detector_with_multimaster(std::vector<::dsn::rpc_address> &meta_servers,
std::function<void()> &&master_disconnected_callback,
std::function<void()> &&master_connected_callback);
~slave_failure_detector_with_multimaster(void);
virtual ~slave_failure_detector_with_multimaster() {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly annotate overrides of virtual functions or virtual destructors with exactly one of an override or (less frequently) final specifier. Do not use virtual when declaring an override. Rationale: A function or destructor marked override or final that is not an override of a base class virtual function will not compile, and this helps catch common errors. The specifiers serve as documentation; if no specifier is present, the reader has to check all ancestors of the class in question to determine if the function or destructor is virtual or not.

@neverchanje neverchanje merged commit 9261ca1 into XiaoMi:master Feb 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants