-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Make sure Functional algorithms check types during conversion #275
base: main
Are you sure you want to change the base?
Conversation
@@ -136,15 +137,35 @@ namespace k4FWCore { | |||
if constexpr (std::is_same_v<EDM4hepType, const podio::CollectionBase*>) { | |||
inputMap.push_back(&get(handle, thisClass, Gaudi::Hive::currentContext())); | |||
} else { | |||
podio::CollectionBase* in = handle.get()->get(); | |||
inputMap.push_back(static_cast<EDM4hepType*>(in)); | |||
podio::CollectionBase* in = handle.get()->get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not entirely sure, but I think in the isVectorLike
branch that we take, we do not deal with possible issues the same way we do in the other branch. It looks like the check that is necessary for cases which have been added via the PodioDataSvc
/ PodioInput
is not caught here and we just let the exception that comes from this line propagate upwards(?).
Should the same handling as below be here as well?
thisClass->name(), | ||
fmt::format( | ||
"Failed to cast collection {} to the required type {}, the type of the collection is {}", | ||
handle.objKey(), typeid(EDM4hepType).name(), in ? in->getTypeName() : "[undetermined]"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With some more work, we could also get the demangled name (as below) here, going via typeid
and name
was the quickest for now. the EDM4hepType::typeName
fails here because EDM4hepType
can still be a podio::CollectionBase
. With a few more lines of code getting the actual type out should be possible.
BEGINRELEASENOTES
ENDRELEASENOTES
Fixes #274
static_cast
s need these checks?