From d71edf024d761682fbe841f6a8803b79c2d04fb7 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Mon, 28 Oct 2024 16:01:43 +0100 Subject: [PATCH 1/3] Fix DataHandle::get() for non-collection types and remove dead code --- k4FWCore/include/k4FWCore/DataHandle.h | 59 +++++++++----------------- 1 file changed, 21 insertions(+), 38 deletions(-) diff --git a/k4FWCore/include/k4FWCore/DataHandle.h b/k4FWCore/include/k4FWCore/DataHandle.h index 92b48c4f..ef130970 100644 --- a/k4FWCore/include/k4FWCore/DataHandle.h +++ b/k4FWCore/include/k4FWCore/DataHandle.h @@ -22,11 +22,8 @@ #include "k4FWCore/DataWrapper.h" #include "k4FWCore/PodioDataSvc.h" -#include "Gaudi/Algorithm.h" #include "GaudiKernel/DataObjectHandle.h" -#include "edm4hep/Constants.h" - #include #include @@ -68,8 +65,6 @@ template class DataHandle : public DataObjectHandle> private: ServiceHandle m_eds; - bool m_isGoodType{false}; - bool m_isCollection{false}; T* m_dataPtr; }; @@ -108,40 +103,28 @@ DataHandle::DataHandle(const std::string& descriptor, Gaudi::DataHandle::Mode * static cast: we do not need the checks of the dynamic cast for every access! */ template const T* DataHandle::get() { - DataObject* dataObjectp = nullptr; - auto sc = m_eds->retrieveObject(DataObjectHandle>::fullKey().key(), dataObjectp); - - if (sc.isSuccess()) { - if (!m_isGoodType && !m_isCollection) { - // only do this once (if both are false after this, we throw exception) - m_isGoodType = nullptr != dynamic_cast*>(dataObjectp); - if (!m_isGoodType) { - auto* tmp = dynamic_cast*>(dataObjectp); - if (tmp != nullptr) { - m_isCollection = nullptr != dynamic_cast(tmp->collectionBase()); - } - } - } - if (m_isGoodType) { - return static_cast*>(dataObjectp)->getData(); - } else if (m_isCollection) { - // The reader does not know the specific type of the collection. So we need a reinterpret_cast if the handle was - // created by the reader. - auto* tmp = static_cast*>(dataObjectp); - return reinterpret_cast(tmp->collectionBase()); - } else { - // When a functional has pushed a std::shared_ptr into the store - auto ptr = static_cast>*>(dataObjectp); - if (ptr) { - return static_cast(ptr->getData().get()); - } - std::string errorMsg("The type provided for " + DataObjectHandle>::pythonRepr() + - " is different from the one of the object in the store."); - throw GaudiException(errorMsg, "wrong product type", StatusCode::FAILURE); - } + DataObject* dataObjectp; + auto sc = m_eds->retrieveObject(DataObjectHandle>::fullKey().key(), dataObjectp); + + if (sc.isFailure()) { + std::string msg("Could not retrieve product " + DataObjectHandle>::pythonRepr()); + throw GaudiException(msg, "wrong product name", StatusCode::FAILURE); + } + bool isGoodType = nullptr != dynamic_cast*>(dataObjectp); + if (isGoodType) { + return static_cast*>(dataObjectp)->getData(); + } + + // When a functional has pushed a std::unique_ptr into the store + // We wrap it inside constexpr because if the handle has a type that is not a collection + // then the static_cast will fail at compile time + if constexpr (std::is_base_of_v) { + auto ptr = static_cast>*>(dataObjectp); + return static_cast(ptr->getData().get()); } - std::string msg("Could not retrieve product " + DataObjectHandle>::pythonRepr()); - throw GaudiException(msg, "wrong product name", StatusCode::FAILURE); + std::string errorMsg("The type provided for " + DataObjectHandle>::pythonRepr() + + " is different from the one of the object in the store."); + throw GaudiException(errorMsg, "wrong product type", StatusCode::FAILURE); } //--------------------------------------------------------------------------- From ebb9e98ce99f5882b68e3fdffeb06833f3c67209 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 29 Oct 2024 08:51:41 +0100 Subject: [PATCH 2/3] Improve exception --- k4FWCore/include/k4FWCore/DataHandle.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/k4FWCore/include/k4FWCore/DataHandle.h b/k4FWCore/include/k4FWCore/DataHandle.h index ef130970..6d148226 100644 --- a/k4FWCore/include/k4FWCore/DataHandle.h +++ b/k4FWCore/include/k4FWCore/DataHandle.h @@ -26,6 +26,7 @@ #include #include +#include /** * Specialisation of the Gaudi DataHandle @@ -123,8 +124,8 @@ template const T* DataHandle::get() { return static_cast(ptr->getData().get()); } std::string errorMsg("The type provided for " + DataObjectHandle>::pythonRepr() + - " is different from the one of the object in the store."); - throw GaudiException(errorMsg, "wrong product type", StatusCode::FAILURE); + " is different from the one of the object in the store " + typeid(*dataObjectp).name()); + throw std::runtime_error(errorMsg); } //--------------------------------------------------------------------------- From aba11ff08ede709ed6161c5fb64d87448488ba46 Mon Sep 17 00:00:00 2001 From: jmcarcell Date: Tue, 29 Oct 2024 08:55:48 +0100 Subject: [PATCH 3/3] Fix pre-commit --- k4FWCore/include/k4FWCore/DataHandle.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k4FWCore/include/k4FWCore/DataHandle.h b/k4FWCore/include/k4FWCore/DataHandle.h index 6d148226..8dfd31af 100644 --- a/k4FWCore/include/k4FWCore/DataHandle.h +++ b/k4FWCore/include/k4FWCore/DataHandle.h @@ -25,8 +25,8 @@ #include "GaudiKernel/DataObjectHandle.h" #include -#include #include +#include /** * Specialisation of the Gaudi DataHandle