Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implement support for Event::getByToken for a GenericHandle #32620

Merged
merged 1 commit into from Jan 12, 2021
Merged

Implement support for Event::getByToken for a GenericHandle #32620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 8 additions & 4 deletions FWCore/Framework/interface/GenericHandle.h
View file
Open in desktop
Expand Up @@ -125,12 +125,16 @@ namespace edm {

///Specialize the Event's getByLabel method to work with a Handle<GenericObject>
template <>
bool edm::Event::getByLabel<GenericObject>(std::string const& label,
std::string const& productInstanceName,
Handle<GenericObject>& result) const;
bool Event::getByLabel<GenericObject>(std::string const& label,
std::string const& productInstanceName,
Handle<GenericObject>& result) const;

template <>
bool edm::Event::getByLabel(edm::InputTag const& tag, Handle<GenericObject>& result) const;
bool Event::getByLabel<GenericObject>(InputTag const& tag, Handle<GenericObject>& result) const;

///Specialize the Event's getByToken method to work with a Handle<GenericObject>
template <>
bool Event::getByToken<GenericObject>(EDGetToken token, Handle<GenericObject>& result) const;

} // namespace edm
#endif
22 changes: 18 additions & 4 deletions FWCore/Framework/src/GenericHandle.cc
View file
Open in desktop
Expand Up @@ -44,9 +44,9 @@ namespace edm {

///Specialize the getByLabel method to work with a Handle<GenericObject>
template <>
bool edm::Event::getByLabel<GenericObject>(std::string const& label,
std::string const& productInstanceName,
Handle<GenericObject>& result) const {
bool Event::getByLabel<GenericObject>(std::string const& label,
std::string const& productInstanceName,
Handle<GenericObject>& result) const {
BasicHandle bh = provRecorder_.getByLabel_(
TypeID(result.type().typeInfo()), label, productInstanceName, std::string(), moduleCallingContext_);
convert_handle(std::move(bh), result); // throws on conversion error
Expand All @@ -58,7 +58,7 @@ namespace edm {
}

template <>
bool edm::Event::getByLabel<GenericObject>(edm::InputTag const& tag, Handle<GenericObject>& result) const {
bool Event::getByLabel<GenericObject>(InputTag const& tag, Handle<GenericObject>& result) const {
if (tag.process().empty()) {
return this->getByLabel(tag.label(), tag.instance(), result);
} else {
Expand All @@ -73,4 +73,18 @@ namespace edm {
return false;
}

///Specialize the Event's getByToken method to work with a Handle<GenericObject>
template <>
bool Event::getByToken<GenericObject>(EDGetToken token, Handle<GenericObject>& result) const {
result.clear();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @fwyzard mentioned in mattermost, the implementations of the getByLabel() specializations have diverged from the generic getByLabel(), whereas the added getByToken() specialization follows the structure of the generic getByToken(). @Dr15Jones would you agree it would be good to update the getByLabel() specializations (in a separate PR unless @fwyzard would be willing to do that here as well)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the differences are not actually functional (I can't really tell with a cursory look) than an update to make it consistent would make sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only possibly functional difference I see is that the generic getByLabel() has this result.clear() call, while the specializations above do not have it.

BasicHandle bh =
provRecorder_.getByToken_(TypeID(result.type().typeInfo()), PRODUCT_TYPE, token, moduleCallingContext_);
convert_handle(std::move(bh), result); // throws on conversion error
if (UNLIKELY(result.failedToGet())) {
return false;
}
addToGotBranchIDs(*result.provenance());
return true;
}

} // namespace edm