From 83a7c23c0f3cdde37476f5c34f38054b5b54b9eb Mon Sep 17 00:00:00 2001 From: Rafal Bielski <rafal.bielski@cern.ch> Date: Tue, 1 Dec 2020 19:40:20 +0100 Subject: [PATCH] TrigOutputHandling: clang-tidy suggestions and small improvements Apply a set of small changes and slight refactoring of the code driven mainly by clang-tidy warnings. The addressed warnings (not always all cases, only where reasonable): * use `= default` to define a trivial destructor [modernize-use-equals-default] * use `using` instead of `typedef` [modernize-use-using] * implicit conversions of pointers to `bool` [readability-implicit-bool-conversion] * implicit conversion from `bool` to `char` [readability-implicit-bool-conversion] * pass by value and use `std::move` [modernize-pass-by-value] * use nullptr [modernize-use-nullptr] * all parameters should be named in a function [readability-named-parameter] * implement the "[rule of five](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-five)" for `BareDataBucket` [cppcoreguidelines-special-member-functions] * implement the "[rule of zero](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-zero)" for `TriggerEDMSerialiserTool::Address` * local copy of a variable never modified -> avoid the copy [performance-unnecessary-copy-initialization] * some methods could be made static member functions or anonymous-namespace functions [readability-convert-member-functions-to-static] * constructor does not initialize some fields [cppcoreguidelines-pro-type-member-init] * use `empty()` instead of `size()==0` [readability-container-size-empty] * some improvements to memory allocation, avoid `new` if possible [cppcoreguidelines-owning-memory] * do not use 'else' after 'return' [readability-else-after-return] * function has a definition with different parameter names [readability-inconsistent-declaration-parameter-name] * use a const reference instead of copy as loop variable [performance-for-range-copy] * use `dynamic_cast` instead of `static_cast` to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast] * use single-character overload of `string::find` when searching for a single character [performance-faster-string-find] * `const` values in declarations do not affect the signature of a function, so they should not be put there [readability-avoid-const-params-in-decls] --- .../TrigOutputHandling/HLTResultMTMaker.h | 9 +- .../TrigOutputHandling/HLTResultMTMakerTool.h | 2 +- .../ITriggerBitsMakerTool.h | 2 +- .../TrigOutputHandling/TriggerBitsMakerTool.h | 6 +- .../TrigOutputHandling/src/BareDataBucket.h | 19 ++- .../src/DecisionSummaryMakerAlg.cxx | 7 +- .../src/DecisionSummaryMakerAlg.h | 2 +- .../TrigOutputHandling/src/HLTEDMCreator.cxx | 10 +- .../TrigOutputHandling/src/HLTEDMCreator.h | 2 +- .../src/HLTEDMCreatorAlg.cxx | 11 +- .../TrigOutputHandling/src/HLTEDMCreatorAlg.h | 3 +- .../src/HLTResultMTMaker.cxx | 8 +- .../src/HLTResultMTMakerAlg.cxx | 2 - .../src/HLTResultMTMakerAlg.h | 2 +- .../src/StreamTagMakerTool.cxx | 28 ++-- .../src/StreamTagMakerTool.h | 6 +- .../src/TriggerBitsMakerTool.cxx | 4 +- .../src/TriggerEDMDeserialiserAlg.cxx | 122 ++++++++++++------ .../src/TriggerEDMDeserialiserAlg.h | 56 ++------ .../src/TriggerEDMSerialiserTool.cxx | 62 ++++----- .../src/TriggerEDMSerialiserTool.h | 35 ++--- .../src/TruncationAnalysisAlg.cxx | 8 +- .../test/schema_evolution_test.cxx | 16 ++- 23 files changed, 194 insertions(+), 228 deletions(-) diff --git a/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/HLTResultMTMaker.h b/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/HLTResultMTMaker.h index db3b1dfb8dc5..dd076e7ee60d 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/HLTResultMTMaker.h +++ b/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/HLTResultMTMaker.h @@ -25,7 +25,7 @@ public: /// Standard constructor HLTResultMTMaker(const std::string& type, const std::string& name, const IInterface* parent); /// Standard destructor - virtual ~HLTResultMTMaker(); + virtual ~HLTResultMTMaker() override = default; // ------------------------- IStateful methods ------------------------------- virtual StatusCode initialize() override; @@ -63,6 +63,11 @@ private: this, "MonTool", "", "Monitoring tool" }; + /// Handle to JobOptionsSvc used to retrieve the DataFlowConfig property + ServiceHandle<Gaudi::Interfaces::IOptionsSvc> m_jobOptionsSvc { + this, "JobOptionsSvc", "JobOptionsSvc", + "Job options service to retrieve DataFlowConfig" + }; /// Extra enabled ROBs Gaudi::Property<std::vector<uint32_t>> m_extraEnabledROBs { this, "ExtraEnabledROBs", {}, @@ -75,8 +80,6 @@ private: }; // ------------------------- Other private members --------------------------- - /// Handle to JobOptionsSvc used to retrieve the DataFlowConfig property - ServiceHandle<Gaudi::Interfaces::IOptionsSvc> m_jobOptionsSvc; /// List of enabled ROBs retrieved during initialisation std::set<uint32_t> m_enabledROBs; /// List of enabled SubDets retrieved during initialisation diff --git a/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/HLTResultMTMakerTool.h b/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/HLTResultMTMakerTool.h index 7027790ee7ef..5b5e2235dde0 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/HLTResultMTMakerTool.h +++ b/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/HLTResultMTMakerTool.h @@ -19,7 +19,7 @@ public: virtual StatusCode fill( HLT::HLTResultMT& resultToFill, const EventContext& ctx ) const = 0; - virtual ~HLTResultMTMakerTool() override {} + virtual ~HLTResultMTMakerTool() override = default; }; #endif // TRIGOUTPUTHANDLING_HLTRESULTMTMAKERTOOL_H diff --git a/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/ITriggerBitsMakerTool.h b/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/ITriggerBitsMakerTool.h index 03618a31db72..8ec429785d47 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/ITriggerBitsMakerTool.h +++ b/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/ITriggerBitsMakerTool.h @@ -22,7 +22,7 @@ public: boost::dynamic_bitset<uint32_t>& rerun, const EventContext& ctx) const = 0; - virtual ~ITriggerBitsMakerTool() override {} + virtual ~ITriggerBitsMakerTool() override = default; }; #endif // TRIGOUTPUTHANDLING_ITRIGGERBITSMAKERTOOL_H diff --git a/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/TriggerBitsMakerTool.h b/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/TriggerBitsMakerTool.h index a9d5c5fd7f87..141335146bf4 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/TriggerBitsMakerTool.h +++ b/Trigger/TrigSteer/TrigOutputHandling/TrigOutputHandling/TriggerBitsMakerTool.h @@ -18,7 +18,7 @@ class TriggerBitsMakerTool : public extends<AthAlgTool, HLTResultMTMakerTool, ITriggerBitsMakerTool> { public: TriggerBitsMakerTool(const std::string& type, const std::string& name, const IInterface* parent); - virtual ~TriggerBitsMakerTool() override; + virtual ~TriggerBitsMakerTool() override = default; virtual StatusCode fill( HLT::HLTResultMT& resultToFill, const EventContext& ctx ) const override; @@ -55,10 +55,10 @@ private: Gaudi::Property<std::map<std::string, uint32_t>> m_extraChainToBit { this, "ExtraChainToBit", {}, "Special case and testing purposes hard-coded chain-to-bit mappings to use in addition to those from the HLT menu."}; - typedef std::map< TrigCompositeUtils::DecisionID, uint32_t> ChainToBitMap; + using ChainToBitMap = std::map< TrigCompositeUtils::DecisionID, uint32_t>; ChainToBitMap m_mapping; //!< Mapping of each chain's hash ID to its chain counter - uint32_t m_largestBit; //!< Largest chain counter hence largest bit needed to be stored in result bitmap + uint32_t m_largestBit{0}; //!< Largest chain counter hence largest bit needed to be stored in result bitmap }; #endif // TRIGOUTPUTHANDLING_TRIGGERBITSMAKERTOOL_H diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/BareDataBucket.h b/Trigger/TrigSteer/TrigOutputHandling/src/BareDataBucket.h index fdb304ec8ae4..f3e9445a0480 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/BareDataBucket.h +++ b/Trigger/TrigSteer/TrigOutputHandling/src/BareDataBucket.h @@ -15,14 +15,19 @@ class BareDataBucket: public DataBucketBase { public: BareDataBucket() = delete; - BareDataBucket( void * data, CLID clid, const RootType& type ) - : m_data(data), m_clid(clid), m_type( type ){} + BareDataBucket( void * data, CLID clid, RootType type ) + : m_data(data), m_clid(clid), m_type( std::move(type) ){} - virtual ~BareDataBucket() { - if ( m_data ) + virtual ~BareDataBucket() override { + if ( m_data != nullptr ) m_type.Destruct( m_data ); } + BareDataBucket(const BareDataBucket&) = delete; + BareDataBucket(BareDataBucket&&) = delete; + BareDataBucket& operator=(const BareDataBucket&) = delete; + BareDataBucket& operator=(BareDataBucket&&) = delete; + // DataObject overrides virtual const CLID& clID() const override { return m_clid; @@ -40,13 +45,13 @@ public: using DataBucketBase::cast; virtual void* cast (CLID clid, - SG::IRegisterTransient* , + SG::IRegisterTransient* /*irt*/, bool isConst = true) override { return ( m_clid == clid and isConst ) ? m_data : nullptr; } virtual void* cast (const std::type_info& tinfo, - SG::IRegisterTransient* , + SG::IRegisterTransient* /*irt*/, bool isConst = true) override { return ( tinfo == m_type.TypeInfo() and isConst ) ? m_data : nullptr; } @@ -58,7 +63,7 @@ public: virtual void lock() override { /*not lockable I think */ }; private: - void* m_data = 0; + void* m_data = nullptr; CLID m_clid = 0; RootType m_type; }; diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/DecisionSummaryMakerAlg.cxx b/Trigger/TrigSteer/TrigOutputHandling/src/DecisionSummaryMakerAlg.cxx index 531fc13566e0..1a574dc702ac 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/DecisionSummaryMakerAlg.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/src/DecisionSummaryMakerAlg.cxx @@ -7,8 +7,6 @@ DecisionSummaryMakerAlg::DecisionSummaryMakerAlg(const std::string& name, ISvcLocator* pSvcLocator) : AthReentrantAlgorithm(name, pSvcLocator) {} -DecisionSummaryMakerAlg::~DecisionSummaryMakerAlg() {} - StatusCode DecisionSummaryMakerAlg::initialize() { renounceArray( m_finalDecisionKeys ); ATH_CHECK( m_finalDecisionKeys.initialize() ); @@ -71,7 +69,7 @@ StatusCode DecisionSummaryMakerAlg::execute(const EventContext& context) const { thisCollFilter->second.begin(), thisCollFilter->second.end(), std::inserter(passingFinalIDs, passingFinalIDs.begin() ) ); // should be faster than remove_if - if (passingFinalIDs.size() == 0) { + if (passingFinalIDs.empty()) { continue; } @@ -148,10 +146,9 @@ StatusCode DecisionSummaryMakerAlg::execute(const EventContext& context) const { // in events which are accepted by one ore more chains. bool filterStatus = true; if (m_setFilterStatus) { - filterStatus = (allPassingFinalIDs.size() > 0); + filterStatus = (not allPassingFinalIDs.empty()); } setFilterPassed(filterStatus, context ); return StatusCode::SUCCESS; } - diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/DecisionSummaryMakerAlg.h b/Trigger/TrigSteer/TrigOutputHandling/src/DecisionSummaryMakerAlg.h index 7b050aac8d3a..14b72bd3871b 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/DecisionSummaryMakerAlg.h +++ b/Trigger/TrigSteer/TrigOutputHandling/src/DecisionSummaryMakerAlg.h @@ -19,7 +19,7 @@ class DecisionSummaryMakerAlg : public AthReentrantAlgorithm { public: DecisionSummaryMakerAlg(const std::string& name, ISvcLocator* pSvcLocator); - virtual ~DecisionSummaryMakerAlg() override; + virtual ~DecisionSummaryMakerAlg() override = default; virtual StatusCode initialize() override; virtual StatusCode execute(const EventContext& context) const override; diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreator.cxx b/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreator.cxx index 2915265ca876..1ebbc05f0eff 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreator.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreator.cxx @@ -24,8 +24,8 @@ StatusCode HLTEDMCreator::initHandles( const HandlesGroup<T>& handles ) { renounceArray( handles.views ); // the case w/o reading from views, both views handles and collection in views should be empty - if ( handles.views.size() == 0 ) { - ATH_CHECK( handles.in.size() == 0 ); + if ( handles.views.empty() ) { + ATH_CHECK( handles.in.empty() ); } else { // the case with views, for every output we expect an input View and an input collection inside that View ATH_CHECK( handles.out.size() == handles.in.size() ); @@ -167,7 +167,7 @@ template<typename T> StatusCode HLTEDMCreator::viewsMerge( ViewContainer const& views, const SG::ReadHandleKey<T>& inViewKey, EventContext const& context, T & output ) const { - typedef typename T::base_value_type type_in_container; + using type_in_container = typename T::base_value_type; StoreGateSvc* sg = evtStore().operator->(); // why the get() method is returing a null ptr is a puzzle, we have to use this ugly call to operator instead of it ATH_CHECK( sg != nullptr ); ViewHelper::ViewMerger merger( sg, msg() ); @@ -178,7 +178,7 @@ StatusCode HLTEDMCreator::viewsMerge( ViewContainer const& views, const SG::Rea StatusCode HLTEDMCreator::fixLinks() const { - if ( m_fixLinks.size() == 0 ) { + if ( m_fixLinks.value().empty() ) { ATH_MSG_DEBUG("fixLinks: No collections defined for this tool"); return StatusCode::SUCCESS; } @@ -269,7 +269,7 @@ StatusCode HLTEDMCreator::createIfMissing( const EventContext& context, const Co for (size_t i = 0; i < handles.out.size(); ++i) { SG::WriteHandleKey<T> writeHandleKey = handles.out.at(i); - if ( handles.views.size() == 0 ) { // no merging will be needed + if ( handles.views.empty() ) { // no merging will be needed // Note: This is correct. We are testing if we can read, and if we cannot then we write. // What we write will either be a dummy (empty) container, or be populated from N in-View collections. SG::ReadHandle<T> readHandle( writeHandleKey.key() ); diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreator.h b/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreator.h index 95cb1bae0678..ac1b9b56717e 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreator.h +++ b/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreator.h @@ -87,7 +87,7 @@ class HLTEDMCreator: public extends<AthAlgTool, IHLTOutputTool> { const std::string& name, const IInterface* parent ); - virtual ~HLTEDMCreator(){} + virtual ~HLTEDMCreator() override = default; virtual StatusCode createOutput(const EventContext& context) const override; virtual StatusCode initialize() override; diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreatorAlg.cxx b/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreatorAlg.cxx index fd6e5b680214..6839877a2012 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreatorAlg.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreatorAlg.cxx @@ -9,24 +9,15 @@ HLTEDMCreatorAlg::HLTEDMCreatorAlg(const std::string& name, ISvcLocator* pSvcLoc { } -HLTEDMCreatorAlg::~HLTEDMCreatorAlg() -{ -} - StatusCode HLTEDMCreatorAlg::initialize() { ATH_CHECK( m_tools.retrieve() ); return StatusCode::SUCCESS; } -StatusCode HLTEDMCreatorAlg::finalize() -{ - return StatusCode::SUCCESS; -} - StatusCode HLTEDMCreatorAlg::execute(const EventContext& context) const { - for ( auto& t: m_tools ) { + for ( const auto& t: m_tools ) { ATH_CHECK( t->createOutput( context ) ); } return StatusCode::SUCCESS; diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreatorAlg.h b/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreatorAlg.h index 41c6b4ca08e4..46a7a37ccf0e 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreatorAlg.h +++ b/Trigger/TrigSteer/TrigOutputHandling/src/HLTEDMCreatorAlg.h @@ -17,11 +17,10 @@ class HLTEDMCreatorAlg : public AthReentrantAlgorithm { public: HLTEDMCreatorAlg(const std::string& name, ISvcLocator* pSvcLocator); - virtual ~HLTEDMCreatorAlg() override; + virtual ~HLTEDMCreatorAlg() override = default; virtual StatusCode initialize() override; virtual StatusCode execute(const EventContext& context) const override; - virtual StatusCode finalize() override; private: ToolHandleArray<IHLTOutputTool> m_tools{ this, "OutputTools", {}, "Tools that generate output"}; diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMaker.cxx b/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMaker.cxx index f4d9754b0b23..c153a02a3847 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMaker.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMaker.cxx @@ -50,13 +50,7 @@ namespace { // Standard constructor // ============================================================================= HLTResultMTMaker::HLTResultMTMaker(const std::string& type, const std::string& name, const IInterface* parent) - : AthAlgTool(type, name, parent), - m_jobOptionsSvc("JobOptionsSvc", name) {} - -// ============================================================================= -// Standard destructor -// ============================================================================= -HLTResultMTMaker::~HLTResultMTMaker() {} + : AthAlgTool(type, name, parent) {} // ============================================================================= // Implementation of IStateful::initialize diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMakerAlg.cxx b/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMakerAlg.cxx index 17fae8a7e6be..3a094bb496be 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMakerAlg.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMakerAlg.cxx @@ -7,8 +7,6 @@ HLTResultMTMakerAlg::HLTResultMTMakerAlg(const std::string& name, ISvcLocator* pSvcLocator) : AthReentrantAlgorithm(name, pSvcLocator) {} -HLTResultMTMakerAlg::~HLTResultMTMakerAlg() {} - StatusCode HLTResultMTMakerAlg::initialize() { ATH_CHECK( m_resultMaker.retrieve() ); return StatusCode::SUCCESS; diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMakerAlg.h b/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMakerAlg.h index e02b400dec6c..f3a22d19565d 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMakerAlg.h +++ b/Trigger/TrigSteer/TrigOutputHandling/src/HLTResultMTMakerAlg.h @@ -15,7 +15,7 @@ class HLTResultMTMakerAlg : public AthReentrantAlgorithm { public: HLTResultMTMakerAlg(const std::string& name, ISvcLocator* pSvcLocator); - virtual ~HLTResultMTMakerAlg() override; + virtual ~HLTResultMTMakerAlg() override = default; virtual StatusCode initialize() override; virtual StatusCode execute(const EventContext& context) const override; diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/StreamTagMakerTool.cxx b/Trigger/TrigSteer/TrigOutputHandling/src/StreamTagMakerTool.cxx index cab9fec63227..e7eaefdfa892 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/StreamTagMakerTool.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/src/StreamTagMakerTool.cxx @@ -11,15 +11,18 @@ using namespace TrigCompositeUtils; -// ============================================================================= - -StreamTagMakerTool::StreamTagMakerTool( const std::string& type, const std::string& name, const IInterface* parent ): - base_class( type, name, parent ) -{} +namespace { + std::string formatStreamTagInfo(const StreamTagMakerTool::StreamTagInfo& info) { + std::ostringstream ss; + ss << "[" << std::get<0>(info) << ", " << std::get<1>(info) << ", " << std::get<2>(info) << ", " << std::get<3>(info) << "]"; + return ss.str(); + } +} // ============================================================================= -StreamTagMakerTool::~StreamTagMakerTool() {} +StreamTagMakerTool::StreamTagMakerTool( const std::string& type, const std::string& name, const IInterface* parent ): + base_class( type, name, parent ) {} // ============================================================================= @@ -141,7 +144,7 @@ StatusCode StreamTagMakerTool::fill( HLT::HLTResultMT& resultToFill, const Event const std::vector<StreamTagInfo>& streams = mappingIter->second; for (const StreamTagInfo& streamTagInfo : streams) { - auto [st_name, st_type, obeysLB, forceFullEvent] = streamTagInfo; + const auto& [st_name, st_type, obeysLB, forceFullEvent] = streamTagInfo; ATH_MSG_DEBUG("Chain " << HLT::Identifier( chain ) << " accepted event into stream " << st_type << "_" << st_name << " (obeysLB=" << obeysLB << ", forceFullEvent=" << forceFullEvent << ")"); std::set<uint32_t> robs; @@ -209,14 +212,3 @@ StatusCode StreamTagMakerTool::fillPEBInfoMap(std::unordered_map<DecisionID, PEB } // Loop over decision containers return StatusCode::SUCCESS; } - -// ============================================================================= - -std::string -StreamTagMakerTool::formatStreamTagInfo (const StreamTagInfo& info) const -{ - std::ostringstream ss; - ss << "[" << std::get<0>(info) << ", " << std::get<1>(info) << ", " << std::get<2>(info) << ", " << std::get<3>(info) << "]"; - return ss.str(); -} - diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/StreamTagMakerTool.h b/Trigger/TrigSteer/TrigOutputHandling/src/StreamTagMakerTool.h index 6291d9724608..b31af48bdae1 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/StreamTagMakerTool.h +++ b/Trigger/TrigSteer/TrigOutputHandling/src/StreamTagMakerTool.h @@ -26,7 +26,7 @@ class StreamTagMakerTool : public extends<AthAlgTool, HLTResultMTMakerTool> { public: StreamTagMakerTool(const std::string& type, const std::string& name, const IInterface* parent); - virtual ~StreamTagMakerTool() override; + virtual ~StreamTagMakerTool() override = default; virtual StatusCode fill( HLT::HLTResultMT& resultToFill, const EventContext& ctx ) const override; @@ -35,11 +35,9 @@ public: virtual StatusCode finalize() override; /// Type describing StreamTag information needed by the tool: {name, type, obeysLumiBlock, forceFullEventBuilding} - typedef std::tuple<std::string, std::string, bool, bool> StreamTagInfo; + using StreamTagInfo = std::tuple<std::string, std::string, bool, bool>; private: - std::string formatStreamTagInfo (const StreamTagInfo& info) const; - SG::ReadHandleKey<TrigConf::HLTMenu> m_hltMenuKey{"DetectorStore+HLTTriggerMenu"}; SG::ReadHandleKey<TrigCompositeUtils::DecisionContainer> m_finalChainDecisions {this, "ChainDecisions", "HLTNav_Summary", diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/TriggerBitsMakerTool.cxx b/Trigger/TrigSteer/TrigOutputHandling/src/TriggerBitsMakerTool.cxx index e9212abad358..649719934a91 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/TriggerBitsMakerTool.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/src/TriggerBitsMakerTool.cxx @@ -11,8 +11,6 @@ TriggerBitsMakerTool::TriggerBitsMakerTool(const std::string& type, const std::string& name, const IInterface* parent) : base_class(type, name, parent){} -TriggerBitsMakerTool::~TriggerBitsMakerTool() {} - StatusCode TriggerBitsMakerTool::initialize() { ATH_CHECK( m_finalChainDecisions.initialize() ); @@ -115,7 +113,7 @@ StatusCode TriggerBitsMakerTool::getBits(boost::dynamic_bitset<uint32_t>& passRa } else if (decisionObject->name() == "HLTPrescaled") { HLTPrescaled = decisionObject; } - if (HLTPassRaw && HLTPrescaled) { + if (HLTPassRaw != nullptr && HLTPrescaled != nullptr) { break; } } diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMDeserialiserAlg.cxx b/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMDeserialiserAlg.cxx index d7a5a53be823..2a194819a45e 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMDeserialiserAlg.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMDeserialiserAlg.cxx @@ -28,7 +28,7 @@ class TriggerEDMDeserialiserAlg::WritableAuxStore : public SG::AuxStoreInternal { public: - WritableAuxStore() {} + WritableAuxStore() = default; using SG::AuxStoreInternal::addVector; }; @@ -44,10 +44,10 @@ namespace { const std::type_info* getElementType ( const std::string& tname, std::string& elementTypeName ) { TClass* cls = TClass::GetClass( tname.c_str() ); - if ( !cls ) return nullptr; + if ( cls == nullptr ) return nullptr; TVirtualCollectionProxy* prox = cls->GetCollectionProxy(); - if ( !prox ) return nullptr; - if ( prox->GetValueClass() ) { + if ( prox == nullptr ) return nullptr; + if ( prox->GetValueClass() != nullptr ) { elementTypeName = prox->GetValueClass()->GetName(); return prox->GetValueClass()->GetTypeInfo(); } @@ -68,12 +68,73 @@ namespace { } +/** + * Collection of helper functions for raw pointer operations on the bytestream payload + * + * Most functions can be constexpr if the compiler implements ConstexprIterator (P0858R0) + * Tested it works in clang9+ regardless of --std flag and in gcc10+ only with --std=c++20 + * TODO: Remove the C++ version checks when the release is built with --std=c++20 or newer + */ +namespace PayloadHelpers { + using TDA = TriggerEDMDeserialiserAlg; + + /// CLID of the collection stored in the next fragment + #if __cpp_lib_array_constexpr >= 201811L || __clang_major__ >= 9 + constexpr + #endif + CLID collectionCLID(TDA::PayloadIterator start) { + return *( start + TDA::CLIDOffset ); + } + + /// Length of the serialised name payload + #if __cpp_lib_array_constexpr >= 201811L || __clang_major__ >= 9 + constexpr + #endif + size_t nameLength(TDA::PayloadIterator start) { + return *( start + TDA::NameLengthOffset ); + } + + /// Size in bytes of the buffer that is needed to decode next fragment data content + #if __cpp_lib_array_constexpr >= 201811L || __clang_major__ >= 9 + constexpr + #endif + size_t dataSize(TDA::PayloadIterator start) { + return *( start + TDA::NameOffset + nameLength(start) ); + } + + /** + * Returns starting point of the next fragment, can be == end() + * + * Intended to be used like this: start = advance(start); if ( start != data.end() )... decode else ... done + **/ + #if __cpp_lib_array_constexpr >= 201811L || __clang_major__ >= 9 + constexpr + #endif + TDA::PayloadIterator toNextFragment(TDA::PayloadIterator start) { + return start + (*start); // point ahead by the number of words pointed to by start iterator + } + + /// String description of the collection stored in the next fragment, returns persistent type name and the SG key + std::vector<std::string> collectionDescription(TDA::PayloadIterator start) { + StringSerializer ss; + std::vector<std::string> labels; + ss.deserialize( start + TDA::NameOffset, start + TDA::NameOffset + nameLength(start), labels ); + return labels; + } + + /// Copies fragment to the buffer, no size checking, use @c dataSize to do so + void toBuffer(TDA::PayloadIterator start, char* buffer) { + // move to the beginning of the buffer memory + TDA::PayloadIterator dataStart = start + TDA::NameOffset + nameLength(start) + 1 /*skip size*/; + // we rely on continuous memory layout of std::vector ... + std::memcpy( buffer, &(*dataStart), dataSize(start) ); + } +} + TriggerEDMDeserialiserAlg::TriggerEDMDeserialiserAlg(const std::string& name, ISvcLocator* pSvcLocator) : AthReentrantAlgorithm(name, pSvcLocator) {} -TriggerEDMDeserialiserAlg::~TriggerEDMDeserialiserAlg() {} - StatusCode TriggerEDMDeserialiserAlg::initialize() { ATH_CHECK( m_resultKey.initialize() ); ATH_CHECK( m_clidSvc.retrieve() ); @@ -90,8 +151,11 @@ StatusCode TriggerEDMDeserialiserAlg::finalize() { StatusCode TriggerEDMDeserialiserAlg::execute(const EventContext& context) const { auto resultHandle = SG::makeHandle( m_resultKey, context ); - if ( resultHandle.isValid() ) - ATH_MSG_DEBUG("Obtained HLTResultMT " << m_resultKey.key() ); + if ( not resultHandle.isValid() ) { + ATH_MSG_ERROR("Failed to obtain HLTResultMT with key " << m_resultKey.key()); + return StatusCode::FAILURE; + } + ATH_MSG_DEBUG("Obtained HLTResultMT with key " << m_resultKey.key()); const Payload* dataptr = nullptr; // TODO: check if there are use cases where result may be not available in some events and this is not an issue at all @@ -106,13 +170,13 @@ StatusCode TriggerEDMDeserialiserAlg::execute(const EventContext& context) const StatusCode TriggerEDMDeserialiserAlg::deserialise( const Payload* dataptr ) const { size_t buffSize = m_initialSerialisationBufferSize; - std::unique_ptr<char[]> buff( new char[buffSize] ); + std::unique_ptr<char[]> buff = std::make_unique<char[]>(buffSize); // returns a char* buffer that is at minimum as large as specified in the argument - auto resize = [&]( size_t neededSize ) { + auto resize = [&buffSize, &buff]( const size_t neededSize ) -> void { if ( neededSize > buffSize ) { buffSize = neededSize; - buff.reset( new char[buffSize] ); + buff = std::make_unique<char[]>(buffSize); } }; @@ -127,21 +191,21 @@ StatusCode TriggerEDMDeserialiserAlg::deserialise( const Payload* dataptr ) c PayloadIterator start = dataptr->begin(); while ( start != dataptr->end() ) { fragmentCount++; - const CLID clid{ collectionCLID( start ) }; + const CLID clid{ PayloadHelpers::collectionCLID( start ) }; std::string transientTypeName; ATH_CHECK( m_clidSvc->getTypeNameOfID( clid, transientTypeName ) ); - const std::vector<std::string> descr{ collectionDescription( start ) }; + const std::vector<std::string> descr{ PayloadHelpers::collectionDescription( start ) }; ATH_CHECK( descr.size() == 2 ); std::string persistentTypeName{ descr[0] }; const std::string key{ descr[1] }; - const size_t bsize{ dataSize( start ) }; + const size_t bsize{ PayloadHelpers::dataSize( start ) }; ATH_MSG_DEBUG( "" ); ATH_MSG_DEBUG( "fragment: " << fragmentCount << " type: "<< transientTypeName << " persistent type: " << persistentTypeName << " key: " << key << " size: " << bsize ); resize( bsize ); - toBuffer( start, buff.get() ); + PayloadHelpers::toBuffer( start, buff.get() ); - start = toNextFragment( start ); // point the start to the next chunk, irrespectively of what happens in deserialisation below + start = PayloadHelpers::toNextFragment( start ); // point the start to the next chunk, irrespectively of what happens in deserialisation below RootType classDesc = RootType::ByNameNoQuiet( persistentTypeName ); ATH_CHECK( classDesc.IsComplete() ); @@ -263,7 +327,8 @@ StatusCode TriggerEDMDeserialiserAlg::checkSanity( const std::string& transientT if ( count == 0 ) { ATH_MSG_ERROR( "Could not recognise the kind of container " << transientTypeName ); return StatusCode::FAILURE; - } else if (count > 1 ) { + } + if (count > 1 ) { ATH_MSG_ERROR( "Ambiguous container kind deduced from the transient type name " << transientTypeName ); ATH_MSG_ERROR( "Recognised type as: " << (isxAODInterfaceContainer ?" xAOD Interface Context":"" ) @@ -276,26 +341,6 @@ StatusCode TriggerEDMDeserialiserAlg::checkSanity( const std::string& transientT } -size_t TriggerEDMDeserialiserAlg::nameLength( TriggerEDMDeserialiserAlg::PayloadIterator start ) const { - return *( start + NameLengthOffset); -} - -std::vector<std::string> TriggerEDMDeserialiserAlg::collectionDescription( TriggerEDMDeserialiserAlg::PayloadIterator start ) const { - StringSerializer ss; - std::vector<std::string> labels; - ss.deserialize( start + NameOffset, start + NameOffset + nameLength(start), labels ); - return labels; -} -size_t TriggerEDMDeserialiserAlg::dataSize( TriggerEDMDeserialiserAlg::PayloadIterator start ) const { - return *( start + NameOffset + nameLength( start ) ); -} - -void TriggerEDMDeserialiserAlg::toBuffer( TriggerEDMDeserialiserAlg::PayloadIterator start, char* buffer ) const { - // move to the beginning of the buffer memory - PayloadIterator dataStart = start + NameOffset + nameLength(start) + 1 /*skip size*/; - // we rely on continuous memory layout of std::vector ... - std::memcpy( buffer, &(*dataStart), dataSize( start ) ); -} void TriggerEDMDeserialiserAlg::add_bs_streamerinfos(){ std::string extStreamerInfos = "bs-streamerinfos.root"; @@ -313,8 +358,9 @@ void TriggerEDMDeserialiserAlg::add_bs_streamerinfos(){ TStreamerInfo* inf = dynamic_cast<TStreamerInfo*>(infObj); inf->BuildCheck(); TClass *cl = inf->GetClass(); - if (cl) + if (cl != nullptr) { ATH_MSG_DEBUG( "external TStreamerInfo for " << cl->GetName() << " checksum: " << std::hex << inf->GetCheckSum() ); + } } } diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMDeserialiserAlg.h b/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMDeserialiserAlg.h index e0c03e0a506b..ba43f5e77c08 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMDeserialiserAlg.h +++ b/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMDeserialiserAlg.h @@ -32,13 +32,14 @@ class TriggerEDMDeserialiserAlg : public AthReentrantAlgorithm { public: class WritableAuxStore; - enum Offsets { - CLIDOffset = 1, - NameLengthOffset = 2, - NameOffset = 3 - }; + using Payload = std::vector<uint32_t>; + using PayloadIterator = Payload::const_iterator; + static constexpr size_t CLIDOffset = 1; + static constexpr size_t NameLengthOffset = 2; + static constexpr size_t NameOffset = 3; + TriggerEDMDeserialiserAlg(const std::string& name, ISvcLocator* pSvcLocator); - virtual ~TriggerEDMDeserialiserAlg() override; + virtual ~TriggerEDMDeserialiserAlg() override = default; virtual StatusCode initialize() override; virtual StatusCode execute(const EventContext& context) const override; @@ -62,45 +63,6 @@ private: std::unique_ptr<TList> m_streamerInfoList; - typedef std::vector<uint32_t> Payload; - typedef std::vector<uint32_t>::const_iterator PayloadIterator; - - /** - * returns starting point of the next fragment, can be == end() - * intended to be used like this: start = advance(start); if ( start != data.end() )... decode else ... done - **/ - inline PayloadIterator toNextFragment( PayloadIterator start ) const { - return start + (*start) ; // point ahead by the number of words pointed to by start iterator - } - /** - * CLID of the collection stored in the next fragment - **/ - inline CLID collectionCLID( PayloadIterator start ) const { - return *( start + CLIDOffset ); - } - /** - * Length of the serialised name payload - **/ - size_t nameLength( TriggerEDMDeserialiserAlg::PayloadIterator start ) const; - - /** - * string description of the collection stored in the next fragment, - * returns persistent type name and the SG key - **/ - std::vector<std::string> collectionDescription( PayloadIterator start ) const; - - /** - * size of the buffer that is needed to decode next fragment data content - * @warning measured in bytes - **/ - size_t dataSize( PayloadIterator start ) const; - - /** - * copies fragment to the buffer, no size checking, use above to do so - **/ - void toBuffer( PayloadIterator start, char* buffer ) const; - - /** * Performs actual deserialisation loop */ @@ -110,13 +72,13 @@ private: * Handle decoration */ StatusCode deserialiseDynAux( const std::string& transientTypeName, const std::string& persistentTypeName, const std::string& decorationName, - void* data, WritableAuxStore* currentAuxStore, SG::AuxVectorBase* interface ) const; + void* obj, WritableAuxStore* currentAuxStore, SG::AuxVectorBase* interface ) const; /** * Checker for data integrity, one and only one of the passed booleans can be true, else FAILURE is returned and relevant diagnostics printed */ - StatusCode checkSanity( const std::string& tn, bool isxAODInterfaceContainer, bool isxAODAuxContainer, bool isDecoration, bool isTPContainer ) const; + StatusCode checkSanity( const std::string& transientTypeName, bool isxAODInterfaceContainer, bool isxAODAuxContainer, bool isDecoration, bool isTPContainer ) const; diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMSerialiserTool.cxx b/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMSerialiserTool.cxx index 18a291418dd1..4aefbbcd405c 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMSerialiserTool.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMSerialiserTool.cxx @@ -32,8 +32,6 @@ TriggerEDMSerialiserTool::TriggerEDMSerialiserTool( const std::string& type, const IInterface* parent ) : base_class( type, name, parent ) {} -TriggerEDMSerialiserTool::~TriggerEDMSerialiserTool() {} - StatusCode TriggerEDMSerialiserTool::initialize() { // Initialise tools and services ATH_CHECK( m_serializerSvc.retrieve() ); @@ -147,22 +145,24 @@ StatusCode TriggerEDMSerialiserTool::addCollectionToSerialise(const std::string& if (!allVars.empty()) { std::set<std::string> variableNames; boost::split( variableNames, allVars, [](const char c){ return c == '.'; } ); - for ( auto el: variableNames ) { - ATH_MSG_DEBUG( " \"" << el << "\"" ); + if (msgLvl(MSG::DEBUG)) { + for ( const auto& el: variableNames ) { + ATH_MSG_DEBUG( " \"" << el << "\"" ); + } } sel.selectAux( variableNames ); } - addressVec.emplace_back( transientType, persistentType, clid, key, moduleIdVec, Address::xAODAux, sel ); + addressVec.push_back( {transientType, persistentType, clid, key, moduleIdVec, Address::Category::xAODAux, sel} ); } else { - addressVec.emplace_back( transientType, persistentType, clid, key, moduleIdVec, Address::xAODInterface, xAOD::AuxSelection() ); + addressVec.push_back( {transientType, persistentType, clid, key, moduleIdVec, Address::Category::xAODInterface} ); } } else { // an old T/P type - addressVec.emplace_back( transientType, persistentType, clid, key, moduleIdVec, Address::OldTP, xAOD::AuxSelection() ); + addressVec.push_back( {transientType, persistentType, clid, key, moduleIdVec, Address::Category::OldTP} ); } return StatusCode::SUCCESS; } -StatusCode TriggerEDMSerialiserTool::makeHeader(const Address& address, std::vector<uint32_t>& buffer ) const { +StatusCode TriggerEDMSerialiserTool::makeHeader(const Address& address, std::vector<uint32_t>& buffer ) { buffer.push_back(0); // fragment size placeholder buffer.push_back( address.clid ); // type info via CLID @@ -244,19 +244,18 @@ StatusCode TriggerEDMSerialiserTool::serialiseDynAux( DataObject* dObj, const Ad if ( mem == nullptr or sz == 0 ) { ATH_MSG_ERROR( "Serialisation of " << address.persType <<"#" << address.key << "."<< decorationName << " unsuccessful" ); return StatusCode::FAILURE; - } else { - ATH_MSG_DEBUG( "Serialised " << address.persType <<"#" << address.key << "."<< decorationName << " memory size " << sz ); } + ATH_MSG_DEBUG( "Serialised " << address.persType <<"#" << address.key << "."<< decorationName << " memory size " << sz ); std::vector<uint32_t> fragment; - Address auxAddress = { typeName, cls->GetName(), clid, decorationName, address.moduleIdVec, Address::xAODDecoration }; + Address auxAddress { typeName, cls->GetName(), clid, decorationName, address.moduleIdVec, Address::Category::xAODDecoration }; ATH_CHECK( makeHeader( auxAddress, fragment ) ); ATH_CHECK( fillPayload( mem, sz, fragment ) ); fragment[0] = fragment.size(); - if ( mem ) delete [] static_cast<const char*>( mem ); + if ( mem != nullptr ) delete [] static_cast<const char*>( mem ); buffer.insert( buffer.end(), fragment.begin(), fragment.end() ); ++nDynWritten; @@ -282,7 +281,7 @@ StatusCode TriggerEDMSerialiserTool::serialiseContainer( void* data, const Addre std::vector<uint32_t> fragment; ATH_CHECK( makeHeader( address, fragment ) ); ATH_CHECK( fillPayload( mem, sz, fragment ) ); - if ( mem ) delete [] static_cast<const char*>( mem ); + if ( mem != nullptr ) delete [] static_cast<const char*>( mem ); ATH_MSG_DEBUG( address.transType << "#" << address.key << " Fragment size :" << fragment.size()*sizeof(uint32_t) << " bytes"); @@ -305,7 +304,7 @@ StatusCode TriggerEDMSerialiserTool::serialisexAODAuxContainer( void* data, static const RootType interface_c = RootType::ByNameNoQuiet( "xAOD::AuxContainerBase" ); void* data_interface = classDesc.Cast (interface_c, data, true); - if (data_interface) { + if (data_interface != nullptr) { const xAOD::AuxContainerBase* store = reinterpret_cast<const xAOD::AuxContainerBase*> (data_interface); copy = classDesc.Construct(); void* copy_interface = classDesc.Cast (interface, copy, true); @@ -340,7 +339,7 @@ StatusCode TriggerEDMSerialiserTool::serialiseTPContainer( void* data, const Add ATH_MSG_DEBUG("TP Container, converting from: " << address.transType << " to " << address.persType ); std::string converterPersistentType; void * persistent = m_tpTool->convertTP( address.transType, data, converterPersistentType ); - ATH_CHECK( persistent != 0 ); + ATH_CHECK( persistent != nullptr ); ATH_CHECK ( converterPersistentType == address.persType ); ATH_CHECK( serialiseContainer( persistent, address, buffer ) ); @@ -361,16 +360,16 @@ StatusCode TriggerEDMSerialiserTool::serialise( const Address& address, std::vec } ATH_MSG_DEBUG("Obtained raw pointer " << rawptr ); - if ( address.category == Address::xAODInterface ) { + if ( address.category == Address::Category::xAODInterface ) { return serialiseContainer( rawptr, address, buffer ); - } else if ( address.category == Address::xAODAux ) { + } + if ( address.category == Address::Category::xAODAux ) { return serialisexAODAuxContainer( rawptr, address, buffer, evtStore ); - } else if ( address.category == Address::OldTP ) { + } + if ( address.category == Address::Category::OldTP ) { return serialiseTPContainer( rawptr, address, buffer ); - } else { - ATH_MSG_ERROR("Unknown Address category - neither of xAODInterface, xAODAux, OldTP"); - return StatusCode::FAILURE; - } + } + ATH_MSG_ERROR("Unknown Address category - neither of xAODInterface, xAODAux, OldTP"); return StatusCode::FAILURE; } @@ -382,7 +381,7 @@ StatusCode TriggerEDMSerialiserTool::fill( HLT::HLTResultMT& resultToFill, const return StatusCode::FAILURE; } - SGImplSvc* evtStore = static_cast<SGImplSvc*>(Atlas::getExtendedEventContext(ctx).proxy()); + SGImplSvc* evtStore = dynamic_cast<SGImplSvc*>(Atlas::getExtendedEventContext(ctx).proxy()); ATH_CHECK( evtStore != nullptr ); // Map storing information to be written out in case of truncation for each module @@ -420,11 +419,11 @@ StatusCode TriggerEDMSerialiserTool::fill( HLT::HLTResultMT& resultToFill, const if (resultToFill.getTruncatedModuleIds().count(id)==0) { ATH_MSG_DEBUG("Module " << id << " payload after inserting " << address.persTypeName() << " has " << resultToFill.getSerialisedData().at(id).size()*sizeof(uint32_t) << " bytes"); - truncationInfoMap[id].emplace_back(&address, thisFragmentSize, true); + truncationInfoMap[id].push_back({&address, thisFragmentSize, true}); } else { ATH_MSG_WARNING("HLTResult with module ID " << id << " truncated - could not add " << address.persTypeName()); - truncationInfoMap[id].emplace_back(&address, thisFragmentSize, false); + truncationInfoMap[id].push_back({&address, thisFragmentSize, false}); } } } @@ -505,7 +504,7 @@ StatusCode TriggerEDMSerialiserTool::fillDebugInfo(const TruncationInfoMap& trun sizeSum += truncationInfo.size; typeNameVec(*debugInfoThisModule).push_back(truncationInfo.addrPtr->persTypeName()); sizeVec(*debugInfoThisModule).push_back(truncationInfo.size); - isRecordedVec(*debugInfoThisModule).push_back(truncationInfo.recorded); + isRecordedVec(*debugInfoThisModule).push_back(static_cast<char>(truncationInfo.recorded)); if (truncationInfo.recorded && truncationInfo.size > largestRecorded.second) { largestRecorded = {truncationInfo.addrPtr->persTypeName(), truncationInfo.size}; } @@ -549,12 +548,13 @@ StatusCode TriggerEDMSerialiserTool::fillDebugInfo(const TruncationInfoMap& trun return StatusCode::SUCCESS; } -std::string TriggerEDMSerialiserTool::version( const std::string& name ) const { +std::string TriggerEDMSerialiserTool::version( const std::string& name ) { if ( name.find("DataVector") != std::string::npos ) { - size_t start = name.find("_"); - return name.substr( start, name.find(">") - start ); + size_t start = name.find('_'); + return name.substr( start, name.find('>') - start ); + } + if ( name.find('_') != std::string::npos ) { + return name.substr( name.find('_') ); } - if ( name.find("_") != std::string::npos ) - return name.substr( name.find("_") ); return ""; } diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMSerialiserTool.h b/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMSerialiserTool.h index 42a4dade5398..03ff663914a4 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMSerialiserTool.h +++ b/Trigger/TrigSteer/TrigOutputHandling/src/TriggerEDMSerialiserTool.h @@ -42,7 +42,7 @@ class TriggerEDMSerialiserTool: public extends<AthAlgTool, HLTResultMTMakerTool> const std::string& name, const IInterface* parent ); - virtual ~TriggerEDMSerialiserTool(); + virtual ~TriggerEDMSerialiserTool() override = default; virtual StatusCode fill( HLT::HLTResultMT& resultToFill, const EventContext& ctx ) const override; virtual StatusCode initialize() override; @@ -83,33 +83,18 @@ class TriggerEDMSerialiserTool: public extends<AthAlgTool, HLTResultMTMakerTool> * Internal structure to keep configuration organised conveniently **/ struct Address { - enum Category { xAODInterface, xAODAux, OldTP, xAODDecoration, None }; - Address( const std::string& transType_, - const std::string& persType_, - const CLID clid_, - const std::string& key_, - const std::vector<uint16_t> module_={}, - const Category category_ = None, - const xAOD::AuxSelection& sel_ = {} ) - : transType(transType_), - persType(persType_), - clid(clid_), - key(key_), - moduleIdVec(module_), - category(category_), - sel(sel_){} + enum class Category : uint8_t { xAODInterface, xAODAux, OldTP, xAODDecoration, None }; std::string transType; std::string persType; // actual versioned type CLID clid; std::string key; - std::vector<uint16_t> moduleIdVec; + std::vector<uint16_t> moduleIdVec{}; + Category category{Category::None}; + xAOD::AuxSelection sel{}; //!< xAOD dynamic variables selection, relevant only for xAODAux category - Category category; - xAOD::AuxSelection sel = {}; //!< xAOD dynamic variables selection, relevant only for xAODAux category - - const std::string transTypeName() const {return transType+"#"+key;} - const std::string persTypeName() const {return persType+"#"+key;} + std::string transTypeName() const {return transType+"#"+key;} + std::string persTypeName() const {return persType+"#"+key;} }; /** @@ -117,8 +102,6 @@ class TriggerEDMSerialiserTool: public extends<AthAlgTool, HLTResultMTMakerTool> * Internal structure to keep information for truncation debugging **/ struct TruncationInfo { - TruncationInfo(const Address* a, const size_t s, const bool r) - : addrPtr(a), size(s), recorded(r) {} const Address* addrPtr{nullptr}; size_t size{0}; bool recorded{false}; @@ -147,7 +130,7 @@ class TriggerEDMSerialiserTool: public extends<AthAlgTool, HLTResultMTMakerTool> /** * Given the ID of the collection (in address arg) insert basic streaming info into the buffer. */ - StatusCode makeHeader( const TriggerEDMSerialiserTool::Address& address, std::vector<uint32_t>& buffer ) const; + static StatusCode makeHeader( const TriggerEDMSerialiserTool::Address& address, std::vector<uint32_t>& buffer ); /** * Copy bytes from the memory into the buffer converting from char[] to uint32_t[] @@ -200,7 +183,7 @@ class TriggerEDMSerialiserTool: public extends<AthAlgTool, HLTResultMTMakerTool> /** * Obtain version from the actual type name */ - std::string version( const std::string& name ) const; + static std::string version( const std::string& name ); }; diff --git a/Trigger/TrigSteer/TrigOutputHandling/src/TruncationAnalysisAlg.cxx b/Trigger/TrigSteer/TrigOutputHandling/src/TruncationAnalysisAlg.cxx index f92ada5b3901..15c4f45eeb6b 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/src/TruncationAnalysisAlg.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/src/TruncationAnalysisAlg.cxx @@ -9,8 +9,6 @@ namespace { struct CollectionDebugInfo { - CollectionDebugInfo(std::string_view n, const uint32_t s, const bool r) - : name(n), size(s), isRecorded(r) {} std::string_view name; uint32_t size; bool isRecorded; @@ -52,9 +50,9 @@ StatusCode TruncationAnalysisAlg::execute(const EventContext& context) const { // Collect name+size+isRecorded in one structure and sort by descending size std::vector<CollectionDebugInfo> collections; for (size_t i=0; i<typeNameVec(*info).size(); ++i) { - collections.emplace_back(typeNameVec(*info).at(i), - sizeVec(*info).at(i), - static_cast<bool>(isRecordedVec(*info).at(i))); + collections.push_back({typeNameVec(*info).at(i), + sizeVec(*info).at(i), + static_cast<bool>(isRecordedVec(*info).at(i))}); } std::sort(collections.begin(), collections.end(), cmpCollections); diff --git a/Trigger/TrigSteer/TrigOutputHandling/test/schema_evolution_test.cxx b/Trigger/TrigSteer/TrigOutputHandling/test/schema_evolution_test.cxx index bc8a1310390d..3a316b681136 100644 --- a/Trigger/TrigSteer/TrigOutputHandling/test/schema_evolution_test.cxx +++ b/Trigger/TrigSteer/TrigOutputHandling/test/schema_evolution_test.cxx @@ -36,13 +36,15 @@ StatusCode tester( TriggerEDMSerialiserTool* ser) { cluster->setE277(0); // got trivial object to serialise, need to create addresses - TriggerEDMSerialiserTool::Address interfaceAddress( "xAOD::TrigEMClusterContainer", "xAOD::TrigEMClusterContainer_v1", - 1264979038/*clid*/, "HLT_one", {}, - TriggerEDMSerialiserTool::Address::xAODInterface ); - - TriggerEDMSerialiserTool::Address auxAddress( "xAOD::TrigEMClusterAuxContainer", "xAOD::TrigEMClusterAuxContainer_v1", - 1111649561/*clid*/, "HLT_oneAux.", {}, - TriggerEDMSerialiserTool::Address::xAODAux ); + TriggerEDMSerialiserTool::Address interfaceAddress{ + "xAOD::TrigEMClusterContainer", "xAOD::TrigEMClusterContainer_v1", + 1264979038/*clid*/, "HLT_one", {}, + TriggerEDMSerialiserTool::Address::Category::xAODInterface}; + + TriggerEDMSerialiserTool::Address auxAddress{ + "xAOD::TrigEMClusterAuxContainer", "xAOD::TrigEMClusterAuxContainer_v1", + 1111649561/*clid*/, "HLT_oneAux.", {}, + TriggerEDMSerialiserTool::Address::Category::xAODAux}; auto status = ser->serialiseContainer( (void*)em, interfaceAddress, serialisedData ); VALUE( status ) EXPECTED( StatusCode::SUCCESS ); -- GitLab