xAODTracking,xAODCaloEvent : Clang-tidy
Same as : !35828 (closed)
but without having: https://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-control-flow.html which seems to caused more noise than being of help in the previous one....
To give a bit more context this is more or less running
https://gitlab.cern.ch/ATLAS-EGamma/Software/Reconstruction/clang-tidy-checks/-/blob/977ea394888d0e1d093d12b7170c31248d99e7e9/.clang-tidy
with -fix
(so in some sense is also gaining some more experience/feedback on what clang-tidy
does
/ which checks make more sense for us).
Changes should relate now to the following mainly
/scratch/anastopoulos/Athena-git/athena/Event/xAOD/xAODBase/Root/IParticleHelpers.cxx:102:13: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
if( ! copy.size() ) {
~~^~~~~~~~~~~
copy.empty()
/cvmfs/atlas-nightlies.cern.ch/repo/sw/master_Athena_x86_64-centos7-clang9-opt/2020-08-21T2130/Athena/22.0.18/InstallArea/x86_64-centos7-clang9-opt/src/Control/AthContainers/AthContainers/DataVector.h:2271:22: note: method 'DataVector'::empty() defined here
[[nodiscard]] bool empty() const noexcept;
^
/scratch/anastopoulos/Athena-git/athena/Event/xAOD/xAODBase/Root/IParticleHelpers.cxx:154:17: warning: use nullptr [modernize-use-nullptr]
return 0;
^
nullptr
/scratch/anastopoulos/Athena-git/athena/Event/xAOD/xAODCaloEvent/Root/CaloCluster_v1.cxx:34:19: warning: calling a base constructor other than the copy constructor [bugprone-copy-constructor-init]
CaloCluster_v1::CaloCluster_v1(const CaloCluster_v1& other)
/scratch/anastopoulos/Athena-git/athena/Event/xAOD/xAODTracking/Root/TrackSummaryAccessors_v1.cxx:9:13: warning: inclusion of deprecated C++ header 'stdint.h'; consider using 'cstdint' instead [modernize-deprecated-headers]
# include <stdint.h>
^~~~~~~~~~
<cstdint>
e.g
- use
empty()
rather that!size()
for "empty" checks. - nullptr rather than
NULL
/0
(stricly speakingNULL
is more of an issue) - "modern" headers ( rather than <foo.h>)
The bugrpone-copy-constuctor-init leads to the only "non 100% trivial" change. Mainly due to making clear why the suggested change is applicable.
Added
virtual ~IParticle() = default;
// since we have a dtor default or delete
// the others
// AuxElement has copy ctor and
// assignment but not move
IParticle() = default;
IParticle(const IParticle&) = default;
IParticle& operator=(const IParticle&) = default;
IParticle(IParticle&&) = delete;
IParticle& operator=(IParticle&&) = delete;
instead of
virtual ~IParticle() {};
Actually , since the base does not have move and we had a dtor, the moves were already "suppressed" The other methods were actually there (defaulted). So this makes things more explicit.
Since copy ctors of derived classes were using the base defaut ctor (assuming there was not default copy ctor for the base...?) there were warnings since for clang-tidy this falls under "bugprone" (using the "wrong" base ctor ..)
In short I followed this https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-five and prb is few more lines but bit more clear what is "there".