[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
finalor just a simple one use-case class without anyvirtualmethods) 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
virtualmethods) 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= defaultfor trivial destructors. [5][6] - If the class is derived, declare the destructor as
virtual ~MyClass() override;. Theoverridehere 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]. Thevirtualis redundant in this case [10][11] but arguably using bothvirtualandoverridemay 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