From f7044ca22bca9c717a1702f0e0a6f34c10f90bba Mon Sep 17 00:00:00 2001 From: Siarhei Harkusha <Siarhei.Harkusha@cern.ch> Date: Mon, 18 Mar 2019 19:33:54 +0100 Subject: [PATCH] Fix const correctness in Tile containers Some methods of Tile containers, which allow modifying the contents of containers, violate the const correctness. Tile convertors have been migrated to use mutable Tile containers. Methods of Tile containers, which violate the const correctness have been removed. --- .../TileEvent/TileBeamElemCollection.h | 10 ++- .../TileEvent/TileDigitsCollection.h | 15 +++- .../TileEvent/TileEvent/TileHitCollection.h | 9 ++- .../TileEvent/TileRawChannelCollection.h | 10 ++- .../TileEvent/TileRawDataCollection.h | 5 +- .../TileEvent/TileRawDataContainer.h | 6 +- .../TileEvent/TileRawDataContainer.icc | 78 +----------------- .../share/TileDigitsThresholdFilter_test.ref | 10 +-- .../test/TileDigitsThresholdFilter_test.cxx | 81 +++++++------------ .../TileTPCnv/T_TilePoolContainerCnv.h | 29 +++++-- 10 files changed, 98 insertions(+), 155 deletions(-) diff --git a/TileCalorimeter/TileEvent/TileEvent/TileBeamElemCollection.h b/TileCalorimeter/TileEvent/TileEvent/TileBeamElemCollection.h index 353d4998ba7..e828ce9b8c4 100755 --- a/TileCalorimeter/TileEvent/TileEvent/TileBeamElemCollection.h +++ b/TileCalorimeter/TileEvent/TileEvent/TileBeamElemCollection.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration */ #ifndef TileBeamElemCollection_H @@ -29,6 +29,14 @@ public: */ TileBeamElemCollection (const TileBeamElemCollection& other); + /** + * @brief Move constructor. + * @param other Collection to move. + * Move the contents from other collection. + */ + TileBeamElemCollection(TileBeamElemCollection&& other) = default; + + ~TileBeamElemCollection() { } }; diff --git a/TileCalorimeter/TileEvent/TileEvent/TileDigitsCollection.h b/TileCalorimeter/TileEvent/TileEvent/TileDigitsCollection.h index 438793223cd..302003f1df8 100755 --- a/TileCalorimeter/TileEvent/TileEvent/TileDigitsCollection.h +++ b/TileCalorimeter/TileEvent/TileEvent/TileDigitsCollection.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration */ #ifndef TileDigitsCollection_H @@ -38,6 +38,13 @@ public: */ TileDigitsCollection (const TileDigitsCollection& other); + /** + * @brief Move constructor. + * @param other Collection to move. + * Move the contents from other collection. + */ + TileDigitsCollection(TileDigitsCollection&& other) = default; + ~TileDigitsCollection() { } /** @@ -66,10 +73,12 @@ public: */ void setFragExtraWords(const std::vector<uint32_t> & extra) { m_FragExtraWords = extra; - if (extra.size() < 2) m_FragExtraWords.resize(2); } + if (extra.size() < 2) m_FragExtraWords.resize(2); + } void setFragExtraWords(std::vector<uint32_t> && extra) { m_FragExtraWords = std::move(extra); - if (extra.size() < 2) m_FragExtraWords.resize(2); } + if (m_FragExtraWords.size() < 2) m_FragExtraWords.resize(2); + } /** * Get Frag extra words for this collection * @return vector with all words diff --git a/TileCalorimeter/TileEvent/TileEvent/TileHitCollection.h b/TileCalorimeter/TileEvent/TileEvent/TileHitCollection.h index 9411ad69dc6..66dd08838a2 100755 --- a/TileCalorimeter/TileEvent/TileEvent/TileHitCollection.h +++ b/TileCalorimeter/TileEvent/TileEvent/TileHitCollection.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration */ #ifndef TileHitCollection_H @@ -30,6 +30,13 @@ public: */ TileHitCollection (const TileHitCollection& other); + /** + * @brief Move constructor. + * @param other Collection to move. + * Move the contents from other collection. + */ + TileHitCollection(TileHitCollection&& other) = default; + ~TileHitCollection() { } }; diff --git a/TileCalorimeter/TileEvent/TileEvent/TileRawChannelCollection.h b/TileCalorimeter/TileEvent/TileEvent/TileRawChannelCollection.h index 48880bda965..f809980e2a2 100755 --- a/TileCalorimeter/TileEvent/TileEvent/TileRawChannelCollection.h +++ b/TileCalorimeter/TileEvent/TileEvent/TileRawChannelCollection.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration */ #ifndef TileRawChannelCollection_H @@ -60,6 +60,14 @@ public: */ TileRawChannelCollection (const TileRawChannelCollection& other); + /** + * @brief Move constructor. + * @param other Collection to move. + * Move the contents from other collection. + */ + TileRawChannelCollection(TileRawChannelCollection&& other) = default; + + ~TileRawChannelCollection() { } /** diff --git a/TileCalorimeter/TileEvent/TileEvent/TileRawDataCollection.h b/TileCalorimeter/TileEvent/TileEvent/TileRawDataCollection.h index 69bcb22ebd4..315c1af28cc 100755 --- a/TileCalorimeter/TileEvent/TileEvent/TileRawDataCollection.h +++ b/TileCalorimeter/TileEvent/TileEvent/TileRawDataCollection.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration */ //******************************************************************** @@ -53,6 +53,9 @@ template <typename TELEMENT> class TileRawDataCollection : DataVector < TELEMENT > (ownPolicy), m_id(0), m_lvl1Id(0), m_lvl1Type(0), m_detEvType(0), m_rodBCID(0) { this->reserve(48); } + TileRawDataCollection<TELEMENT> (const TileRawDataCollection<TELEMENT>& rhs) = default; + TileRawDataCollection<TELEMENT> (TileRawDataCollection<TELEMENT>&& rhs) = default; + // destructor virtual ~TileRawDataCollection<TELEMENT> () = default; diff --git a/TileCalorimeter/TileEvent/TileEvent/TileRawDataContainer.h b/TileCalorimeter/TileEvent/TileEvent/TileRawDataContainer.h index b24a91400d4..b342641de83 100755 --- a/TileCalorimeter/TileEvent/TileEvent/TileRawDataContainer.h +++ b/TileCalorimeter/TileEvent/TileEvent/TileRawDataContainer.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration */ //******************************************************************** @@ -59,10 +59,6 @@ public: // clear all collections void clear(); - // insert a TileRawData element into a collection. - // this is only to be used by Algorithm-builder - void add (TElement* rc, bool createColl=false, SG::OwnershipPolicy ownPolicy=SG::OWN_ELEMENTS); - inline void push_back(TElement* rc) { add(rc,true); } inline TYPE get_hashType() const { return this->m_hashFunc.type(); } inline UNIT get_unit() const { return this->m_unit; } inline void set_unit(UNIT unit) { m_unit=unit; } diff --git a/TileCalorimeter/TileEvent/TileEvent/TileRawDataContainer.icc b/TileCalorimeter/TileEvent/TileEvent/TileRawDataContainer.icc index 9bd9a408828..52d4b4a1c27 100755 --- a/TileCalorimeter/TileEvent/TileEvent/TileRawDataContainer.icc +++ b/TileCalorimeter/TileEvent/TileEvent/TileRawDataContainer.icc @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration */ // implementation of TileRawDataContainer @@ -90,82 +90,6 @@ void TileRawDataContainer<TCOLLECTION>::clear() return; } -template <typename TCOLLECTION> -void TileRawDataContainer<TCOLLECTION>::add(TElement* rc, bool createColl, - SG::OwnershipPolicy ownPolicy) -{ - if (this->m_hashFunc.max() == 0 && TileCablingService::getInstance()->getTileHWID() != 0) { - // not initialized yet - initialize hash function - initialize(false,m_type); - } - - //if (isLocked()) { - // std::cout << " Can not change TileRawDataContainer anymore, It is locked"<<std::endl; - // return ; - //} - - TCOLLECTION * coll; - - int frag = rc->frag_ID(); - IdentifierHash fragHash = static_cast<IdentifierHash>(m_hashFunc(frag)); - - TContainer_const_iterator it = MyBase::indexFind(fragHash); - - if( it == MyBase::end() ){ // collection doesn't exist - - // do not create collection, because it'll not work anyhow: - // the fact that collection doesn't exist cashed already - // waitng for update of core package - // createColl = false; - - if (createColl) { - - coll = new TCOLLECTION(frag,ownPolicy); - StatusCode sc = this->addCollection(coll,fragHash); - if (sc.isFailure() ) { - - ISvcLocator* svcLoc = Gaudi::svcLocator( ); - IMessageSvc* msgSvc; - sc = svcLoc->service( "MessageSvc", msgSvc ); - if ( sc.isFailure() ) { - std::cout << "TileRawDataContainer ERROR Can not retrieve MessageSvc" << std::endl; - std::cout << "TileRawDataContainer ERROR Can not create collection for frag 0x" << std::hex << frag - << " in container with CLID " << std::dec << this->clID() << std::endl; - } else { - MsgStream log(msgSvc, "TileRawDataContainer"); - log << MSG::ERROR <<" Can not create collection for frag 0x" << MSG::hex << frag - << " in container with CLID " << MSG::dec << this->clID() << endmsg; - } - return ; - } - - } else { - - ISvcLocator* svcLoc = Gaudi::svcLocator( ); - IMessageSvc* msgSvc; - StatusCode sc = svcLoc->service( "MessageSvc", msgSvc ); - if ( sc.isFailure() ) { - std::cout << "TileRawDataContainer ERROR Can not retrieve MessageSvc" << std::endl; - std::cout << "TileRawDataContainer ERROR Collection for frag 0x" << std::hex << frag - << " in container with CLID " << std::dec << this->clID() - << " does not exist " << std::endl; - } else { - MsgStream log(msgSvc, "TileRawDataContainer"); - log << MSG::ERROR <<" Collection for frag 0x" << MSG::hex << frag - << " in container with CLID " << MSG::dec << this->clID() - << " does not exist " << endmsg; - } - return ; - } - } else { // collection exists - - const TCOLLECTION * const_coll = *it; - coll = const_cast<TCOLLECTION *>(const_coll); - } - - coll->push_back(rc); - return ; -} template <typename TCOLLECTION> void TileRawDataContainer<TCOLLECTION>::print() const diff --git a/TileCalorimeter/TileRecAlgs/share/TileDigitsThresholdFilter_test.ref b/TileCalorimeter/TileRecAlgs/share/TileDigitsThresholdFilter_test.ref index 4b32e718f1b..06d5f0d24bd 100644 --- a/TileCalorimeter/TileRecAlgs/share/TileDigitsThresholdFilter_test.ref +++ b/TileCalorimeter/TileRecAlgs/share/TileDigitsThresholdFilter_test.ref @@ -4,12 +4,12 @@ Initializing Gaudi ApplicationMgr using job opts ./TileDigitsThresholdFilter_tes JobOptionsSvc INFO Job options successfully read in from ./TileDigitsThresholdFilter_test_generated.txt ApplicationMgr SUCCESS ==================================================================================================================================== - Welcome to ApplicationMgr (GaudiCoreSvc v27r1p99) - running on karma on Fri Jul 20 15:32:00 2018 + Welcome to ApplicationMgr (GaudiCoreSvc v31r0) + running on pcatl12 on Mon Mar 18 19:21:10 2019 ==================================================================================================================================== ApplicationMgr INFO Successfully loaded modules : StoreGate ApplicationMgr INFO Application Manager Configured successfully -ClassIDSvc INFO getRegistryEntries: read 8597 CLIDRegistry entries for module ALL +ClassIDSvc INFO getRegistryEntries: read 5499 CLIDRegistry entries for module ALL StoreGateSvc DEBUG Property update for OutputLevel : new value = 2 StoreGateSvc DEBUG Service base class initialized successfully StoreGateSvc DEBUG trying to create store SGImplSvc/StoreGateSvc_Impl @@ -54,7 +54,3 @@ TILE => 5/0/1/2/1/1 TILE => 5/0/1/2/2/1 TILE => 5/0/1/2/3/1 TILE => 5/0/1/2/4/1 -StoreGateSvc DEBUG Recorded object @0x1c23240 with key TileDigitsCnt of type TileDigitsContainer(CLID 2925) - in DataObject @0x16cea90 - object not modifiable when retrieved -StoreGateSvc DEBUG Retrieved const pointer to object TileDigitsFiltered of type TileDigitsContainer(CLID 2925) diff --git a/TileCalorimeter/TileRecAlgs/test/TileDigitsThresholdFilter_test.cxx b/TileCalorimeter/TileRecAlgs/test/TileDigitsThresholdFilter_test.cxx index d8708224432..4ceb5db626f 100644 --- a/TileCalorimeter/TileRecAlgs/test/TileDigitsThresholdFilter_test.cxx +++ b/TileCalorimeter/TileRecAlgs/test/TileDigitsThresholdFilter_test.cxx @@ -1,5 +1,5 @@ /* - Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration */ #undef NDEBUG @@ -7,6 +7,7 @@ #include "../src/TileDigitsThresholdFilter.h" #include "TileConditions/ITileCondToolDspThreshold.h" #include "TileEvent/TileDigitsContainer.h" +#include "TileEvent/TileMutableDigitsContainer.h" #include "TileIdentifier/TileHWID.h" #include "TileConditions/TileCablingService.h" @@ -19,6 +20,8 @@ #include "AthenaBaseComps/AthAlgTool.h" #include "StoreGate/StoreGateSvc.h" #include "StoreGate/setupStoreGate.h" +#include "StoreGate/WriteHandle.h" +#include "StoreGate/ReadHandle.h" // ATLAS C++ #include "CxxUtils/make_unique.h" @@ -128,46 +131,37 @@ void test1() { assert( (alg->setProperty("TileCondToolDspThreshold", "TileCondToolDspThresholdMock/TileCondToolDspThresholdMock")).isSuccess() ); assert( (alg->initialize()).isSuccess() ); - - TileDigitsContainer* inputContainer = new TileDigitsContainer(true); - - inputContainer->set_unit(TileRawChannelUnit::OnlineADCcounts); - inputContainer->set_type(TileFragHash::Beam); - inputContainer->set_bsflags(3); - - unsigned int ros = 1; - unsigned int drawer = 1; - for (unsigned int adc = 0; adc < 2; ++adc) { - unsigned int channel = 0; - for (const std::vector<float>& digits : TESTDIGITS) { - HWIdentifier id = tileHWID->adc_id(ros, drawer, channel, adc); - inputContainer->push_back(new TileDigits(id, digits)); - ++channel; - std::cout << ((tileHWID->is_tile(id) ? "TILE" : "NOT TILE")) << " => " << tileHWID->to_string(id) << std::endl; + { + auto digitsContainer = std::make_unique<TileMutableDigitsContainer>(true); + + digitsContainer->set_unit(TileRawChannelUnit::OnlineADCcounts); + digitsContainer->set_type(TileFragHash::Beam); + digitsContainer->set_bsflags(3); + + unsigned int ros = 1; + unsigned int drawer = 1; + for (unsigned int adc = 0; adc < 2; ++adc) { + unsigned int channel = 0; + for (const std::vector<float>& digits : TESTDIGITS) { + HWIdentifier id = tileHWID->adc_id(ros, drawer, channel, adc); + digitsContainer->push_back(new TileDigits(id, digits)); + ++channel; + std::cout << ((tileHWID->is_tile(id) ? "TILE" : "NOT TILE")) << " => " << tileHWID->to_string(id) << std::endl; + } + ++drawer; } - ++drawer; - } - /* - for (const TileDigitsCollection* constInputCollection : *inputContainer) { - if (constInputCollection->identify() == 0x101) { - TileDigitsCollection* inputCollection = const_cast<TileDigitsCollection*>(constInputCollection); - inputCollection->setLvl1Id(10100); - inputCollection->setLvl1Type(10101); - inputCollection->setDetEvType(10102); - inputCollection->setRODBCID(10103); - } + SG::WriteHandle<TileDigitsContainer> digitsCnt("TileDigitsCnt"); + assert(digitsCnt.record(std::move(digitsContainer)).isSuccess()); } - */ - - assert( evtStore->record(inputContainer, "TileDigitsCnt", false).isSuccess() ); - assert( (alg->execute()).isSuccess() ); + SG::ReadHandle<TileDigitsContainer> inputContainer("TileDigitsCnt"); + assert( inputContainer.isValid() ); - const TileDigitsContainer* outputContainer; - assert( evtStore->retrieve(outputContainer, "TileDigitsFiltered").isSuccess() ); + SG::ReadHandle<TileDigitsContainer> outputContainer("TileDigitsFiltered"); + assert( outputContainer.isValid() ); assert( inputContainer->get_unit() == outputContainer->get_unit() ); assert( inputContainer->get_type() == outputContainer->get_type() ); @@ -180,25 +174,6 @@ void test1() { unsigned int drawer = (fragId & 0x3F); unsigned int ros = fragId >> 8; - /* - IdentifierHash fragHash = (inputContainer->hashFunc())(fragId); - const TileDigitsCollection* inputCollection = *(inputContainer->indexFind(fragHash)); - - - assert( inputCollection->getLvl1Id() == outputCollection->getLvl1Id() ); - assert( inputCollection->getLvl1Type() == outputCollection->getLvl1Type() ); - assert( inputCollection->getDetEvType() == outputCollection->getDetEvType() ); - assert( inputCollection->getRODBCID() == outputCollection->getRODBCID() ); - - assert( inputCollection->getFragSize() == outputCollection->getFragSize() ); - assert( inputCollection->getFragExtraWords() == outputCollection->getFragExtraWords() ); - assert( inputCollection->getFragBCID() == outputCollection->getFragBCID() ); - assert( inputCollection->getFragChipHeaderWords() == outputCollection->getFragChipHeaderWords() ); - assert( inputCollection->getFragChipHeaderWordsHigh() == outputCollection->getFragChipHeaderWordsHigh() ); - assert( inputCollection->getFragChipCRCWords() == outputCollection->getFragChipCRCWords() ); - assert( inputCollection->getFragChipCRCWordsHigh() == outputCollection->getFragChipCRCWordsHigh() ); - */ - for (const TileDigits* digits : *outputCollection) { ++nTileDigitsPassedFitler; HWIdentifier adc_id = digits->adc_HWID(); diff --git a/TileCalorimeter/TileSvc/TileTPCnv/TileTPCnv/T_TilePoolContainerCnv.h b/TileCalorimeter/TileSvc/TileTPCnv/TileTPCnv/T_TilePoolContainerCnv.h index 98332371654..b763231e8d4 100644 --- a/TileCalorimeter/TileSvc/TileTPCnv/TileTPCnv/T_TilePoolContainerCnv.h +++ b/TileCalorimeter/TileSvc/TileTPCnv/TileTPCnv/T_TilePoolContainerCnv.h @@ -1,7 +1,7 @@ ///////////////////////// -*- C++ -*- ///////////////////////////// /* - Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration + Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration */ // T_TilePoolContainerCnv.h @@ -15,6 +15,7 @@ #include "AthenaPoolCnvSvc/T_AthenaPoolTPConverter.h" #include "EventContainers/SelectAllObject.h" #include "TileEvent/TileRawDataContainer.h" +#include "TileEvent/TileMutableDataContainer.h" #include <vector> #include <inttypes.h> @@ -26,6 +27,7 @@ public: typedef typename PERS::ElemVector pers_ElemVector; typedef typename PERS::const_iterator pers_const_iterator; typedef typename SelectAllObject<TRANS>::const_iterator trans_const_iterator; + using Collection = typename TRANS::IDENTIFIABLE; T_TilePoolContainerCnv() : m_elementCnv() {} @@ -52,7 +54,6 @@ public: << " - " << bsflags << " " << unit << " " << type << " " << hashType << MSG::dec << " Nelements= " << vec.size() << endmsg; - //trans->clear(); // only remove elements trans->cleanup(); // remove all collections if ( abs(trans->get_hashType()-hashType) > 0xF) { @@ -67,11 +68,27 @@ public: trans->set_type((TileFragHash::TYPE)type); trans->set_bsflags(bsflags); - for( pers_const_iterator it = vec.begin(), - iEnd = vec.end(); - it != iEnd; ++it) { - trans->push_back( m_elementCnv.createTransient(&(*it), log) ); + + auto mutableContainer = std::make_unique<TileMutableDataContainer<TRANS>>(); + if (mutableContainer->status().isFailure()) { + throw std::runtime_error("Failed to initialize Tile mutable Container"); + } + + for(const auto& element : vec) { + if (mutableContainer->push_back(m_elementCnv.createTransient(&element, log)).isFailure()) { + throw std::runtime_error("Failed to add Tile element to Collection"); + } } + + auto hashes = mutableContainer->GetAllCurrentHashes(); + for (auto hash : hashes) { + Collection* coll = mutableContainer->indexFindPtr(hash); + auto newColl = std::make_unique<Collection>(std::move(*coll)); + if (trans->addOrDelete(std::move(newColl), hash).isFailure()) { + throw std::runtime_error("Failed to add Tile collection to Identifiable Container"); + } + } + } /** Converts vector of TRANS::value_type objects to vector of PERS::value_type objects, -- GitLab