[TrigConfiguration] Clean up destructor definitions
There are many changes in this MR but all are trivial and of the same kind. Adjust all classes in the TrigConfiguration domain to follow the destructor definition rules outlined below. I focused on this domain since it is core framework code (so should serve as an example of good code) and involves a lot of inheritance where this is relevant.
The rules followed are:
- If the class is not involved in inheritance and is unlikely to be (e.g. defined as
final
or just a simple one use-case class without anyvirtual
methods) and does not require a custom destructor, do not explicitly declare the destructor. This follows the "rule of zero". [1][2] - If the class is a base class for other classes (or may be and has
virtual
methods) and does not require a custom destructor, declare it asvirtual ~MyClass() = default;
. It is required to be defined to avoid undefined behaviour [3][4] and it is recommended to use= default
for trivial destructors. [5][6] - If the class is derived, declare the destructor as
virtual ~MyClass() override;
. Theoverride
here is a clear additional indication of the inheritance and enforces that the base class has a virtual destructor defined, which prevents a class of bugs [7-10]. Thevirtual
is redundant in this case [10][11] but arguably using bothvirtual
andoverride
may be useful and is actually recommended by ATLAS [12] as well as widespread and standard in the athena repository. - In addition to 3. if no custom implementation is required, declare the destructor as
virtual ~MyClass() override = default;
. This is the most common change in this MR.
On top of everything, adding the override
to destructors uncovers other missing overrides in many classes since it triggers the [-Winconsistent-missing-override]
compilation warning for classes where the override
was incorrectly but consistently missing in multiple methods. Another good reason why this MR improves the code quality. The uncovered missing overrides were fixed in !51739 (merged).
[1] https://en.cppreference.com/w/cpp/language/rule_of_three
[2] https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-zero
[3] https://en.cppreference.com/w/cpp/language/destructor
[4] https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c35-a-base-class-destructor-should-be-either-public-and-virtual-or-protected-and-non-virtual
[5] https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#example-good-6
[6] https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-equals-default.html
[7] https://stackoverflow.com/questions/17923370/override-identifier-after-destructor-in-c11
[8] https://github.com/isocpp/CppCoreGuidelines/pull/1448
[9] https://github.com/isocpp/CppCoreGuidelines/issues/721
[10] https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-override.html
[11] https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rh-override
[12] [redeclare-virtual] in https://atlas-computing.web.cern.ch/atlas-computing/projects/qa/draft_guidelines.html#orgbb16391