From 4795f922d0afc39b75024bb28f1bfe4ce394c563 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Fri, 18 Jan 2019 08:11:46 -0600 Subject: [PATCH 1/6] Functions now return BasicHandle directly Instead of passing BasicHandle in as an argument, we now return it directly from the function. This allowed removal of the default constructor. Attempted to optimize convert_handle. --- DataFormats/Common/interface/BasicHandle.h | 47 +++++++++---------- DataFormats/Common/interface/ConvertHandle.h | 39 ++++++++++----- DataFormats/Common/src/ConvertHandle.cc | 14 ++++-- FWCore/Common/interface/EventBase.h | 4 +- FWCore/Common/interface/LuminosityBlockBase.h | 2 +- FWCore/Common/interface/RunBase.h | 2 +- FWCore/Framework/interface/Event.h | 13 +++-- FWCore/Framework/interface/LuminosityBlock.h | 8 ++-- .../Framework/interface/OccurrenceForOutput.h | 10 ++-- .../Framework/interface/PrincipalGetAdapter.h | 4 +- FWCore/Framework/interface/Run.h | 8 ++-- FWCore/Framework/src/OccurrenceForOutput.cc | 11 ++--- FWCore/Framework/src/Principal.cc | 8 +++- FWCore/Framework/test/Event_t.cpp | 2 +- FWCore/Integration/test/TableTestModules.cc | 3 +- .../src/GetProductCheckerOutputModule.cc | 3 +- .../src/ProvenanceCheckerOutputModule.cc | 3 +- IOPool/Input/test/IOExerciser.cc | 4 +- IOPool/Output/src/RootOutputFile.cc | 5 +- .../SecondaryInput/test/SecondaryProducer.cc | 3 +- IOPool/Streamer/src/StreamSerializer.cc | 3 +- 21 files changed, 100 insertions(+), 96 deletions(-) diff --git a/DataFormats/Common/interface/BasicHandle.h b/DataFormats/Common/interface/BasicHandle.h index 31ca37f8284d4..859f154983bc8 100644 --- a/DataFormats/Common/interface/BasicHandle.h +++ b/DataFormats/Common/interface/BasicHandle.h @@ -42,60 +42,52 @@ namespace edm { class BasicHandle { public: - BasicHandle() : - product_(), - prov_(nullptr) {} + BasicHandle() = delete; - BasicHandle(BasicHandle const& h) : - product_(h.product_), - prov_(h.prov_), - whyFailedFactory_(h.whyFailedFactory_){} + BasicHandle(BasicHandle const& h) = default; BasicHandle(BasicHandle &&h) = default; - - BasicHandle(WrapperBase const* iProd, Provenance const* iProv) : + + BasicHandle(WrapperBase const* iProd, Provenance const* iProv) noexcept(true): product_(iProd), prov_(iProv) { } ///Used when the attempt to get the data failed - BasicHandle(std::shared_ptr const& iWhyFailed): + BasicHandle(std::shared_ptr const& iWhyFailed) noexcept(true) : product_(), prov_(nullptr), whyFailedFactory_(iWhyFailed) {} - ~BasicHandle() {} + ~BasicHandle() = default; - void swap(BasicHandle& other) { + void swap(BasicHandle& other) noexcept(true){ using std::swap; swap(product_, other.product_); std::swap(prov_, other.prov_); swap(whyFailedFactory_,other.whyFailedFactory_); } - BasicHandle& operator=(BasicHandle const& rhs) { - BasicHandle temp(rhs); - this->swap(temp); - return *this; - } + BasicHandle& operator=(BasicHandle&& rhs) = default; + BasicHandle& operator=(BasicHandle const& rhs) = default; - bool isValid() const { + bool isValid() const noexcept(true) { return product_ && prov_; } - bool failedToGet() const { + bool failedToGet() const noexcept(true) { return bool(whyFailedFactory_); } - WrapperBase const* wrapper() const { + WrapperBase const* wrapper() const noexcept(true) { return product_; } - Provenance const* provenance() const { + Provenance const* provenance() const noexcept(true) { return prov_; } - ProductID id() const { + ProductID id() const noexcept(true) { return prov_->productID(); } @@ -103,21 +95,24 @@ namespace edm { return whyFailedFactory_->make(); } - std::shared_ptr const& whyFailedFactory() const { + std::shared_ptr const& whyFailedFactory() const noexcept(true) { return whyFailedFactory_; } - std::shared_ptr& whyFailedFactory() { + std::shared_ptr& whyFailedFactory() noexcept(true) { return whyFailedFactory_; } - void clear() { + void clear() noexcept(true) { product_ = nullptr; prov_ = nullptr; whyFailedFactory_.reset(); } + static BasicHandle makeInvalid() { return BasicHandle(true);} private: + //This is used to create a special invalid BasicHandle + explicit BasicHandle(bool):product_(nullptr) {} WrapperBase const* product_; Provenance const* prov_; std::shared_ptr whyFailedFactory_; @@ -126,7 +121,7 @@ namespace edm { // Free swap function inline void - swap(BasicHandle& a, BasicHandle& b) { + swap(BasicHandle& a, BasicHandle& b) noexcept(true) { a.swap(b); } } diff --git a/DataFormats/Common/interface/ConvertHandle.h b/DataFormats/Common/interface/ConvertHandle.h index e48ab3a1cdc77..5a2c8bfae90fc 100644 --- a/DataFormats/Common/interface/ConvertHandle.h +++ b/DataFormats/Common/interface/ConvertHandle.h @@ -4,38 +4,51 @@ #include "DataFormats/Common/interface/BasicHandle.h" #include "DataFormats/Common/interface/Handle.h" #include "DataFormats/Common/interface/Wrapper.h" +#include "FWCore/Utilities/interface/Likely.h" #include #include +#include namespace edm { namespace handleimpl { - void throwInvalidReference(); void throwConvertTypeError(std::type_info const& expected, std::type_info const& actual); + std::shared_ptr makeInvalidReferenceException(); } // Convert from handle-to-void to handle-to-T template - void convert_handle(BasicHandle && bh, - Handle& result) { - if(bh.failedToGet()) { - Handle h(std::move(bh.whyFailedFactory())); - result = std::move(h); - return; + Handle convert_handle(BasicHandle && bh) noexcept(true) { + if UNLIKELY(bh.failedToGet()) { + return Handle(std::move(bh.whyFailedFactory())); } void const* basicWrapper = bh.wrapper(); - if(basicWrapper == nullptr) { - handleimpl::throwInvalidReference(); + if UNLIKELY(nullptr == basicWrapper) { + return Handle{handleimpl::makeInvalidReferenceException()}; } - if(!(bh.wrapper()->dynamicTypeInfo() == typeid(T))) { + auto wrapper = static_cast const*>(basicWrapper); + + return Handle(wrapper->product(), bh.provenance()); + } + + template + Handle convert_handle_check_type(BasicHandle && bh) { + if UNLIKELY(bh.failedToGet()) { + return Handle(std::move(bh.whyFailedFactory())); + } + void const* basicWrapper = bh.wrapper(); + if UNLIKELY(basicWrapper == nullptr) { + return Handle{handleimpl::makeInvalidReferenceException()}; + } + if UNLIKELY(!(bh.wrapper()->dynamicTypeInfo() == typeid(T))) { handleimpl::throwConvertTypeError(typeid(T), bh.wrapper()->dynamicTypeInfo()); } Wrapper const* wrapper = static_cast const*>(basicWrapper); - - Handle h(wrapper->product(), bh.provenance()); - h.swap(result); + + return Handle(wrapper->product(), bh.provenance()); } + } #endif diff --git a/DataFormats/Common/src/ConvertHandle.cc b/DataFormats/Common/src/ConvertHandle.cc index 61c8ce8fe08b3..c37e73794b255 100644 --- a/DataFormats/Common/src/ConvertHandle.cc +++ b/DataFormats/Common/src/ConvertHandle.cc @@ -1,13 +1,19 @@ #include "DataFormats/Common/interface/ConvertHandle.h" #include "FWCore/Utilities/interface/EDMException.h" +#include "DataFormats/Common/interface/FunctorHandleExceptionFactory.h" namespace edm { namespace handleimpl { - void throwInvalidReference() { - throw Exception(errors::InvalidReference, "NullPointer") - << "edm::BasicHandle has null pointer to Wrapper"; - } + static std::shared_ptr s_invalidRefFactory = makeHandleExceptionFactory([]()->std::shared_ptr { + std::shared_ptr whyFailed = std::make_shared(errors::InvalidReference, "NullPointer"); + *whyFailed << "Handle has null pointer to data product"; + return whyFailed; + }); + std::shared_ptr makeInvalidReferenceException() { + return s_invalidRefFactory; + } + void throwConvertTypeError(std::type_info const& expected, std::type_info const& actual) { throw Exception(errors::LogicError, "TypeMismatch") << "edm::BasicHandle contains a product of type " << actual.name() << ".\n" diff --git a/FWCore/Common/interface/EventBase.h b/FWCore/Common/interface/EventBase.h index a0dbb38ea659a..9c851f184e46f 100644 --- a/FWCore/Common/interface/EventBase.h +++ b/FWCore/Common/interface/EventBase.h @@ -94,7 +94,7 @@ namespace edm { EventBase::getByLabel(InputTag const& tag, Handle& result) const { result.clear(); BasicHandle bh = this->getByLabelImpl(typeid(edm::Wrapper), typeid(T), tag); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); if (result.failedToGet()) { return false; } @@ -106,7 +106,7 @@ namespace edm { EventBase::get(ProductID const& pid, Handle& result) const { result.clear(); BasicHandle bh = this->getImpl(typeid(T), pid); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle_check_type(std::move(bh)); if (result.failedToGet()) { return false; } diff --git a/FWCore/Common/interface/LuminosityBlockBase.h b/FWCore/Common/interface/LuminosityBlockBase.h index 617181e4f48f9..e95e144371971 100644 --- a/FWCore/Common/interface/LuminosityBlockBase.h +++ b/FWCore/Common/interface/LuminosityBlockBase.h @@ -73,7 +73,7 @@ namespace edm { LuminosityBlockBase::getByLabel(const InputTag& tag, Handle& result) const { result.clear(); BasicHandle bh = this->getByLabelImpl(typeid(Wrapper), typeid(T), tag); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); if (result.failedToGet()) { return false; } diff --git a/FWCore/Common/interface/RunBase.h b/FWCore/Common/interface/RunBase.h index cdf7481359514..b8431cd43c3a1 100644 --- a/FWCore/Common/interface/RunBase.h +++ b/FWCore/Common/interface/RunBase.h @@ -60,7 +60,7 @@ namespace edm { RunBase::getByLabel(InputTag const& tag, Handle& result) const { result.clear(); BasicHandle bh = this->getByLabelImpl(typeid(Wrapper), typeid(T), tag); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); if (result.failedToGet()) { return false; } diff --git a/FWCore/Framework/interface/Event.h b/FWCore/Framework/interface/Event.h index dfe3357867e42..8bdefeb30223f 100644 --- a/FWCore/Framework/interface/Event.h +++ b/FWCore/Framework/interface/Event.h @@ -356,7 +356,7 @@ namespace edm { Event::get(ProductID const& oid, Handle& result) const { result.clear(); BasicHandle bh = this->getByProductID_(oid); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle_check_type(std::move(bh)); // throws on conversion error if(result.failedToGet()) { return false; } @@ -371,13 +371,12 @@ namespace edm { BasicHandle bh = this->getByProductID_(oid); if(bh.failedToGet()) { - Handle > temp(makeHandleExceptionFactory([oid]()->std::shared_ptr { + result = Handle >(makeHandleExceptionFactory([oid]()->std::shared_ptr { std::shared_ptr whyFailed = std::make_shared(edm::errors::ProductNotFound); *whyFailed << "get View by ID failed: no product with ID = " << oid <<"\n"; return whyFailed; })); - result.swap(temp); return false; } @@ -535,7 +534,7 @@ namespace edm { Event::getByLabel(InputTag const& tag, Handle& result) const { result.clear(); BasicHandle bh = provRecorder_.getByLabel_(TypeID(typeid(PROD)), tag, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } @@ -550,7 +549,7 @@ namespace edm { Handle& result) const { result.clear(); BasicHandle bh = provRecorder_.getByLabel_(TypeID(typeid(PROD)), label, productInstanceName, emptyString_, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } @@ -579,7 +578,7 @@ namespace edm { Event::getByToken(EDGetToken token, Handle& result) const { result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } @@ -592,7 +591,7 @@ namespace edm { Event::getByToken(EDGetTokenT token, Handle& result) const { result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } diff --git a/FWCore/Framework/interface/LuminosityBlock.h b/FWCore/Framework/interface/LuminosityBlock.h index bc73a736c716f..9d4b3272e3b21 100644 --- a/FWCore/Framework/interface/LuminosityBlock.h +++ b/FWCore/Framework/interface/LuminosityBlock.h @@ -303,7 +303,7 @@ namespace edm { } result.clear(); BasicHandle bh = provRecorder_.getByLabel_(TypeID(typeid(PROD)), label, productInstanceName, emptyString_, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } @@ -319,7 +319,7 @@ namespace edm { } result.clear(); BasicHandle bh = provRecorder_.getByLabel_(TypeID(typeid(PROD)), tag, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } @@ -334,7 +334,7 @@ namespace edm { } result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } @@ -349,7 +349,7 @@ namespace edm { } result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } diff --git a/FWCore/Framework/interface/OccurrenceForOutput.h b/FWCore/Framework/interface/OccurrenceForOutput.h index d699587264203..0cfe2239ca26c 100644 --- a/FWCore/Framework/interface/OccurrenceForOutput.h +++ b/FWCore/Framework/interface/OccurrenceForOutput.h @@ -53,8 +53,8 @@ namespace edm { ProcessHistoryID const& processHistoryID() const; - bool - getByToken(EDGetToken token, TypeID const& typeID, BasicHandle& result) const; + BasicHandle + getByToken(EDGetToken token, TypeID const& typeID) const; template bool @@ -97,9 +97,8 @@ namespace edm { if(!provRecorder_.checkIfComplete()) { principal_get_adapter_detail::throwOnPrematureRead("RunOrLumi", TypeID(typeid(PROD)), token); } - result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } @@ -112,9 +111,8 @@ namespace edm { if(!provRecorder_.checkIfComplete()) { principal_get_adapter_detail::throwOnPrematureRead("RunOrLumi", TypeID(typeid(PROD)), token); } - result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } diff --git a/FWCore/Framework/interface/PrincipalGetAdapter.h b/FWCore/Framework/interface/PrincipalGetAdapter.h index cac56183d15f7..70a450923d02b 100644 --- a/FWCore/Framework/interface/PrincipalGetAdapter.h +++ b/FWCore/Framework/interface/PrincipalGetAdapter.h @@ -365,9 +365,7 @@ namespace edm { typename BasicHandleVec::iterator end = bhv.end(); while (it != end) { - Handle result; - convert_handle(std::move(*it), result); // throws on conversion error - products.push_back(result); + products.push_back(convert_handle(std::move(*it))); ++it; } results.swap(products); diff --git a/FWCore/Framework/interface/Run.h b/FWCore/Framework/interface/Run.h index b963aa6806b48..a50b9d748a962 100644 --- a/FWCore/Framework/interface/Run.h +++ b/FWCore/Framework/interface/Run.h @@ -315,7 +315,7 @@ namespace edm { } result.clear(); BasicHandle bh = provRecorder_.getByLabel_(TypeID(typeid(PROD)), label, productInstanceName, emptyString_, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } @@ -331,7 +331,7 @@ namespace edm { } result.clear(); BasicHandle bh = provRecorder_.getByLabel_(TypeID(typeid(PROD)), tag, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } @@ -346,7 +346,7 @@ namespace edm { } result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } @@ -361,7 +361,7 @@ namespace edm { } result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); - convert_handle(std::move(bh), result); // throws on conversion error + result = convert_handle(std::move(bh)); // throws on conversion error if (result.failedToGet()) { return false; } diff --git a/FWCore/Framework/src/OccurrenceForOutput.cc b/FWCore/Framework/src/OccurrenceForOutput.cc index 52e105275cfca..99e18d2ac59cb 100644 --- a/FWCore/Framework/src/OccurrenceForOutput.cc +++ b/FWCore/Framework/src/OccurrenceForOutput.cc @@ -61,17 +61,16 @@ namespace edm { return provRecorder_.principal().size(); } - bool - OccurrenceForOutput::getByToken(EDGetToken token, TypeID const& typeID, BasicHandle& result) const { - result.clear(); - result = provRecorder_.getByToken_(typeID, PRODUCT_TYPE, token, moduleCallingContext_); + BasicHandle + OccurrenceForOutput::getByToken(EDGetToken token, TypeID const& typeID) const { + auto result = provRecorder_.getByToken_(typeID, PRODUCT_TYPE, token, moduleCallingContext_); if (result.failedToGet()) { - return false; + return result; } if(!provRecorder_.isComplete() && result.wrapper()->isMergeable()) { principal_get_adapter_detail::throwOnPrematureRead("RunOrLumi", typeID, token); } - return true; + return result; } } diff --git a/FWCore/Framework/src/Principal.cc b/FWCore/Framework/src/Principal.cc index 0e7b1b357fc27..c9aed7a137978 100644 --- a/FWCore/Framework/src/Principal.cc +++ b/FWCore/Framework/src/Principal.cc @@ -606,11 +606,15 @@ namespace edm { auto resolution = productResolver->resolveProduct(*this, skipCurrentProcess, sra, mcc); if(resolution.isAmbiguous()) { ambiguous = true; - return BasicHandle(); + //The caller is looking explicitly for this case + // and uses the extra data at the caller to setup the exception + return BasicHandle::makeInvalid(); } auto productData = resolution.data(); if(productData == nullptr) { - return BasicHandle(); + //The caller is looking explicitly for this case + // and uses the extra data at the caller to setup the exception + return BasicHandle::makeInvalid(); } return BasicHandle(productData->wrapper(), &(productData->provenance())); } diff --git a/FWCore/Framework/test/Event_t.cpp b/FWCore/Framework/test/Event_t.cpp index 07f3f97b83ede..9616ee992c59f 100644 --- a/FWCore/Framework/test/Event_t.cpp +++ b/FWCore/Framework/test/Event_t.cpp @@ -701,7 +701,7 @@ void testEvent::getByLabel() { } BasicHandle bh = principal_->getByLabel(PRODUCT_TYPE, TypeID(typeid(edmtest::IntProduct)), "modMulti", "int1", "LATE",nullptr, nullptr, nullptr); - convert_handle(std::move(bh), h); + h = convert_handle(std::move(bh)); CPPUNIT_ASSERT(h->value == 100); BasicHandle bh2(principal_->getByLabel(PRODUCT_TYPE, TypeID(typeid(edmtest::IntProduct)), "modMulti", "int1", "nomatch",nullptr, nullptr, nullptr)); CPPUNIT_ASSERT(!bh2.isValid()); diff --git a/FWCore/Integration/test/TableTestModules.cc b/FWCore/Integration/test/TableTestModules.cc index 3a36afac15e05..9ebd2c8d19c60 100644 --- a/FWCore/Integration/test/TableTestModules.cc +++ b/FWCore/Integration/test/TableTestModules.cc @@ -104,8 +104,7 @@ namespace edmtest { BranchDescription const* branchDescription = product.first; TypeID const& tid = branchDescription->unwrappedTypeID(); EDGetToken const& token = product.second; - BasicHandle bh; - e.getByToken(token, tid, bh); + BasicHandle bh = e.getByToken(token, tid); assert(bh.isValid()); auto examiner = bh.wrapper()->tableExaminer(); assert(examiner); diff --git a/FWCore/Modules/src/GetProductCheckerOutputModule.cc b/FWCore/Modules/src/GetProductCheckerOutputModule.cc index d052f1cd69fce..7edb1d74d1351 100644 --- a/FWCore/Modules/src/GetProductCheckerOutputModule.cc +++ b/FWCore/Modules/src/GetProductCheckerOutputModule.cc @@ -85,8 +85,7 @@ namespace edm { BranchDescription const* branchDescription = product.first; TypeID const& tid = branchDescription->unwrappedTypeID(); EDGetToken const& token = product.second; - BasicHandle bh; - p.getByToken(token, tid, bh); + BasicHandle bh = p.getByToken(token, tid); if(nullptr != bh.provenance() && bh.provenance()->branchDescription().branchID() != branchDescription->branchID()) { throw cms::Exception("BranchIDMissMatch") << "While processing " << id << " getByToken request for " << branchDescription->moduleLabel() << " '" << branchDescription->productInstanceName() << "' " << branchDescription->processName() diff --git a/FWCore/Modules/src/ProvenanceCheckerOutputModule.cc b/FWCore/Modules/src/ProvenanceCheckerOutputModule.cc index b65902a088a5d..98188c67e10cb 100644 --- a/FWCore/Modules/src/ProvenanceCheckerOutputModule.cc +++ b/FWCore/Modules/src/ProvenanceCheckerOutputModule.cc @@ -117,8 +117,7 @@ namespace edm { idToBranchDescriptions[branchID] = branchDescription; TypeID const& tid(branchDescription->unwrappedTypeID()); EDGetToken const& token = product.second; - BasicHandle bh; - e.getByToken(token, tid, bh); + BasicHandle bh = e.getByToken(token, tid); bool cannotFindProductProvenance=false; if(!(bh.provenance() and bh.provenance()->productProvenance())) { missingProductProvenance.insert(branchID); diff --git a/IOPool/Input/test/IOExerciser.cc b/IOPool/Input/test/IOExerciser.cc index d420ab20592ff..82f89ef20d8fb 100644 --- a/IOPool/Input/test/IOExerciser.cc +++ b/IOPool/Input/test/IOExerciser.cc @@ -150,9 +150,7 @@ IOExerciser::write(edm::EventForOutput const& e) for (auto const& product : products_to_use) { - edm::BasicHandle result; - - e.getByToken(product.token(), product.type(), result); + edm::BasicHandle result = e.getByToken(product.token(), product.type()); ctr++; } edm::LogInfo("IOExerciser") << "IOExerciser read out " << ctr << " products."; diff --git a/IOPool/Output/src/RootOutputFile.cc b/IOPool/Output/src/RootOutputFile.cc index c5ee1ad04d31f..43680d821cd7c 100644 --- a/IOPool/Output/src/RootOutputFile.cc +++ b/IOPool/Output/src/RootOutputFile.cc @@ -725,11 +725,10 @@ namespace edm { WrapperBase const* product = nullptr; ProductProvenance const* productProvenance = nullptr; - BasicHandle result; if(getProd) { - bool found = occurrence.getByToken(item.token_, item.branchDescription_->unwrappedTypeID(), result); + BasicHandle result = occurrence.getByToken(item.token_, item.branchDescription_->unwrappedTypeID()); product = result.wrapper(); - if(found && keepProvenance) { + if(result.isValid() && keepProvenance) { productProvenance = result.provenance()->productProvenance(); } if(product == nullptr) { diff --git a/IOPool/SecondaryInput/test/SecondaryProducer.cc b/IOPool/SecondaryInput/test/SecondaryProducer.cc index 6821dc7ed4d5f..ddd857b688a18 100644 --- a/IOPool/SecondaryInput/test/SecondaryProducer.cc +++ b/IOPool/SecondaryInput/test/SecondaryProducer.cc @@ -122,8 +122,7 @@ namespace edm { nullptr, nullptr); assert(bhandle.isValid()); - Handle handle; - convert_handle(std::move(bhandle), handle); + Handle handle = convert_handle(std::move(bhandle)); assert(static_cast(handle->value) == en); // Check that primary source products are retrieved from the same event as the EventAuxiliary diff --git a/IOPool/Streamer/src/StreamSerializer.cc b/IOPool/Streamer/src/StreamSerializer.cc index b9d30c6e4d2ae..3ae127df29072 100644 --- a/IOPool/Streamer/src/StreamSerializer.cc +++ b/IOPool/Streamer/src/StreamSerializer.cc @@ -150,8 +150,7 @@ namespace edm { for(auto const& selection : *selections_) { BranchDescription const& desc = *selection.first; - BasicHandle result; - event.getByToken(selection.second, desc.unwrappedTypeID(), result); + BasicHandle result = event.getByToken(selection.second, desc.unwrappedTypeID()); if(!result.isValid()) { // No product with this ID was put in the event. // Create and write the provenance. From cf0e2e3caac6d20d7cd4b598c7f87301e14ea200 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Fri, 18 Jan 2019 14:28:20 -0600 Subject: [PATCH 2/6] Added ability to do bool value checks to HandleBase --- DataFormats/Common/interface/HandleBase.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/DataFormats/Common/interface/HandleBase.h b/DataFormats/Common/interface/HandleBase.h index bd358d96ff45f..6ce3cce444c4d 100644 --- a/DataFormats/Common/interface/HandleBase.h +++ b/DataFormats/Common/interface/HandleBase.h @@ -113,6 +113,14 @@ namespace edm { std::shared_ptr const& whyFailedFactory() const { return whyFailedFactory_;} + explicit operator bool () const { + return isValid(); + } + + bool operator!() const { + return not isValid(); + } + protected: void const* productStorage() const; From bf61ed63e3cde38c25ec70fc60a0d4473252728f Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Fri, 18 Jan 2019 14:32:38 -0600 Subject: [PATCH 3/6] Added getHandle<> and get<> methods to Run, LuminosityBlock and Event Returns directly the object with which one wants to interact. --- FWCore/Framework/interface/Event.h | 96 ++++++++-- FWCore/Framework/interface/LuminosityBlock.h | 27 +++ .../Framework/interface/OccurrenceForOutput.h | 14 ++ FWCore/Framework/interface/Run.h | 29 +++ FWCore/Framework/test/Event_t.cpp | 179 ++++++++++++++++++ 5 files changed, 324 insertions(+), 21 deletions(-) diff --git a/FWCore/Framework/interface/Event.h b/FWCore/Framework/interface/Event.h index 8bdefeb30223f..7f5b33656d38f 100644 --- a/FWCore/Framework/interface/Event.h +++ b/FWCore/Framework/interface/Event.h @@ -202,6 +202,14 @@ namespace edm { bool getByToken(EDGetTokenT token, Handle& result) const; + template + Handle + getHandle(EDGetTokenT token) const; + + template + PROD const & + get(EDGetTokenT token) const noexcept(false); + // Template member overload to deal with Views. template bool @@ -226,11 +234,17 @@ namespace edm { bool getByToken(EDGetTokenT> token, Handle>& result) const; + template + Handle> + getHandle(EDGetTokenT> token) const; template - void - fillView_(BasicHandle& bh, - Handle >& result) const; + View const & + get(EDGetTokenT> token) const noexcept(false); + + template + Handle > + fillView_(BasicHandle& bh) const; Provenance getProvenance(BranchID const& theID) const; @@ -380,7 +394,7 @@ namespace edm { return false; } - fillView_(bh, result); + result = fillView_(bh); return true; } @@ -535,7 +549,7 @@ namespace edm { result.clear(); BasicHandle bh = provRecorder_.getByLabel_(TypeID(typeid(PROD)), tag, moduleCallingContext_); result = convert_handle(std::move(bh)); // throws on conversion error - if (result.failedToGet()) { + if UNLIKELY(result.failedToGet()) { return false; } addToGotBranchIDs(*result.provenance()); @@ -550,7 +564,7 @@ namespace edm { result.clear(); BasicHandle bh = provRecorder_.getByLabel_(TypeID(typeid(PROD)), label, productInstanceName, emptyString_, moduleCallingContext_); result = convert_handle(std::move(bh)); // throws on conversion error - if (result.failedToGet()) { + if UNLIKELY(result.failedToGet()) { return false; } addToGotBranchIDs(*result.provenance()); @@ -579,7 +593,7 @@ namespace edm { result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); result = convert_handle(std::move(bh)); // throws on conversion error - if (result.failedToGet()) { + if UNLIKELY(result.failedToGet()) { return false; } addToGotBranchIDs(*result.provenance()); @@ -591,26 +605,48 @@ namespace edm { Event::getByToken(EDGetTokenT token, Handle& result) const { result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); - result = convert_handle(std::move(bh)); // throws on conversion error - if (result.failedToGet()) { + result = convert_handle(std::move(bh)); + if UNLIKELY(result.failedToGet()) { return false; } addToGotBranchIDs(*result.provenance()); return true; } + template + Handle + Event::getHandle(EDGetTokenT token) const { + BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); + auto result = convert_handle(std::move(bh)); + if LIKELY(not result.failedToGet()) { + addToGotBranchIDs(*result.provenance()); + } + return result; + } + + template + PROD const& + Event::get(EDGetTokenT token) const noexcept(false) { + BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); + auto result = convert_handle(std::move(bh)); + if LIKELY(not result.failedToGet()) { + addToGotBranchIDs(*result.provenance()); + } + return *result; + } + template bool Event::getByLabel(InputTag const& tag, Handle >& result) const { result.clear(); BasicHandle bh = provRecorder_.getMatchingSequenceByLabel_(TypeID(typeid(ELEMENT)), tag, moduleCallingContext_); - if(bh.failedToGet()) { + if UNLIKELY(bh.failedToGet()) { Handle > h(std::move(bh.whyFailedFactory())); h.swap(result); return false; } - fillView_(bh, result); + result = fillView_(bh); return true; } @@ -621,12 +657,12 @@ namespace edm { Handle >& result) const { result.clear(); BasicHandle bh = provRecorder_.getMatchingSequenceByLabel_(TypeID(typeid(ELEMENT)), moduleLabel, productInstanceName, emptyString_, moduleCallingContext_); - if(bh.failedToGet()) { + if UNLIKELY(bh.failedToGet()) { Handle > h(std::move(bh.whyFailedFactory())); h.swap(result); return false; } - fillView_(bh, result); + result = fillView_(bh); return true; } @@ -641,12 +677,12 @@ namespace edm { Event::getByToken(EDGetToken token, Handle>& result) const { result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(ELEMENT)),ELEMENT_TYPE, token, moduleCallingContext_); - if(bh.failedToGet()) { + if UNLIKELY(bh.failedToGet()) { Handle > h(std::move(bh.whyFailedFactory())); h.swap(result); return false; } - fillView_(bh, result); + result = fillView_(bh); return true; } @@ -655,19 +691,38 @@ namespace edm { Event::getByToken(EDGetTokenT> token, Handle>& result) const { result.clear(); BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(ELEMENT)),ELEMENT_TYPE, token, moduleCallingContext_); - if(bh.failedToGet()) { + if UNLIKELY(bh.failedToGet()) { Handle > h(std::move(bh.whyFailedFactory())); h.swap(result); return false; } - fillView_(bh, result); + result = fillView_(bh); return true; } + template + Handle> + Event::getHandle(EDGetTokenT> token) const { + BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(ELEMENT)),ELEMENT_TYPE, token, moduleCallingContext_); + if UNLIKELY(bh.failedToGet()) { + return Handle >(std::move(bh.whyFailedFactory()));; + } + return fillView_(bh); + } + + template + View const & + Event::get(EDGetTokenT> token) const noexcept(false) { + BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(ELEMENT)),ELEMENT_TYPE, token, moduleCallingContext_); + if UNLIKELY(bh.failedToGet()) { + bh.whyFailedFactory()->make()->raise(); + } + return *fillView_(bh); + } template - void - Event::fillView_(BasicHandle& bh, Handle >& result) const { + Handle > + Event::fillView_(BasicHandle& bh) const { std::vector pointersToElements; FillViewHelperVector helpers; // the following must initialize the @@ -678,8 +733,7 @@ namespace edm { addToGotBranchIDs(*bh.provenance()); gotViews_.push_back(newview); - Handle > h(&*newview, bh.provenance()); - result.swap(h); + return Handle >(newview.get(), bh.provenance()); } // Free functions to retrieve a collection from the Event. diff --git a/FWCore/Framework/interface/LuminosityBlock.h b/FWCore/Framework/interface/LuminosityBlock.h index 9d4b3272e3b21..4b74e23f6f1c9 100644 --- a/FWCore/Framework/interface/LuminosityBlock.h +++ b/FWCore/Framework/interface/LuminosityBlock.h @@ -97,6 +97,14 @@ namespace edm { bool getByToken(EDGetTokenT token, Handle& result) const; + template + Handle + getHandle(EDGetTokenT token) const; + + template + PROD const& + get(EDGetTokenT token) const noexcept(false); + template void @@ -356,6 +364,25 @@ namespace edm { return true; } + template + Handle + LuminosityBlock::getHandle(EDGetTokenT token) const { + if UNLIKELY(!provRecorder_.checkIfComplete()) { + principal_get_adapter_detail::throwOnPrematureRead("Lumi", TypeID(typeid(PROD)), token); + } + BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); + return convert_handle(std::move(bh)); + } + + template + PROD const& + LuminosityBlock::get(EDGetTokenT token) const noexcept(false) { + if UNLIKELY(!provRecorder_.checkIfComplete()) { + principal_get_adapter_detail::throwOnPrematureRead("Lumi", TypeID(typeid(PROD)), token); + } + BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); + return *convert_handle(std::move(bh)); + } template void diff --git a/FWCore/Framework/interface/OccurrenceForOutput.h b/FWCore/Framework/interface/OccurrenceForOutput.h index 0cfe2239ca26c..11edbd8a7f793 100644 --- a/FWCore/Framework/interface/OccurrenceForOutput.h +++ b/FWCore/Framework/interface/OccurrenceForOutput.h @@ -64,6 +64,10 @@ namespace edm { bool getByToken(EDGetTokenT token, Handle& result) const; + template + Handle + getHandle(EDGetTokenT token) const; + Provenance getProvenance(BranchID const& theID) const; @@ -119,6 +123,16 @@ namespace edm { return true; } + template + Handle + OccurrenceForOutput::getHandle(EDGetTokenT token) const { + if(!provRecorder_.checkIfComplete()) { + principal_get_adapter_detail::throwOnPrematureRead("RunOrLumi", TypeID(typeid(PROD)), token); + } + BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); + return convert_handle(std::move(bh)); // throws on conversion error + } + } #endif diff --git a/FWCore/Framework/interface/Run.h b/FWCore/Framework/interface/Run.h index a50b9d748a962..e344672faa446 100644 --- a/FWCore/Framework/interface/Run.h +++ b/FWCore/Framework/interface/Run.h @@ -105,6 +105,15 @@ namespace edm { bool getByToken(EDGetTokenT token, Handle& result) const; + template + Handle + getHandle(EDGetTokenT token) const; + + template + PROD const& + get(EDGetTokenT token) const noexcept(false); + + template void getManyByType(std::vector >& results) const; @@ -368,6 +377,26 @@ namespace edm { return true; } + template + Handle + Run::getHandle(EDGetTokenT token) const { + if(!provRecorder_.checkIfComplete()) { + principal_get_adapter_detail::throwOnPrematureRead("Run", TypeID(typeid(PROD)), token); + } + BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); + return convert_handle(std::move(bh)); + } + + template + PROD const& + Run::get(EDGetTokenT token) const noexcept(false) { + if(!provRecorder_.checkIfComplete()) { + principal_get_adapter_detail::throwOnPrematureRead("Run", TypeID(typeid(PROD)), token); + } + BasicHandle bh = provRecorder_.getByToken_(TypeID(typeid(PROD)),PRODUCT_TYPE, token, moduleCallingContext_); + return *convert_handle(std::move(bh)); + } + template void Run::getManyByType(std::vector >& results) const { diff --git a/FWCore/Framework/test/Event_t.cpp b/FWCore/Framework/test/Event_t.cpp index 9616ee992c59f..4f75b0dfa042a 100644 --- a/FWCore/Framework/test/Event_t.cpp +++ b/FWCore/Framework/test/Event_t.cpp @@ -87,6 +87,8 @@ class testEvent: public CppUnit::TestFixture { CPPUNIT_TEST(emptyEvent); CPPUNIT_TEST(getByLabelFromEmpty); CPPUNIT_TEST(getByTokenFromEmpty); + CPPUNIT_TEST(getHandleFromEmpty); + CPPUNIT_TEST(getFromEmpty); CPPUNIT_TEST(putAnIntProduct); CPPUNIT_TEST(putAndGetAnIntProduct); CPPUNIT_TEST(putAndGetAnIntProductByToken); @@ -95,6 +97,8 @@ class testEvent: public CppUnit::TestFixture { CPPUNIT_TEST(transaction); CPPUNIT_TEST(getByLabel); CPPUNIT_TEST(getByToken); + CPPUNIT_TEST(getHandle); + CPPUNIT_TEST(get_product); CPPUNIT_TEST(getManyByType); CPPUNIT_TEST(printHistory); CPPUNIT_TEST(deleteProduct); @@ -108,6 +112,8 @@ class testEvent: public CppUnit::TestFixture { void emptyEvent(); void getByLabelFromEmpty(); void getByTokenFromEmpty(); + void getHandleFromEmpty(); + void getFromEmpty(); void putAnIntProduct(); void putAndGetAnIntProduct(); void putAndGetAnIntProductByToken(); @@ -116,6 +122,8 @@ class testEvent: public CppUnit::TestFixture { void transaction(); void getByLabel(); void getByToken(); + void getHandle(); + void get_product(); void getManyByType(); void printHistory(); void deleteProduct(); @@ -505,6 +513,30 @@ void testEvent::getByTokenFromEmpty() { CPPUNIT_ASSERT_THROW(*nonesuch, cms::Exception); } +void testEvent::getHandleFromEmpty() { + InputTag inputTag("moduleLabel", "instanceName"); + + IntConsumer consumer( std::vector{1,inputTag}); + consumer.updateLookup(InEvent, principal_->productLookup(),false); + assert(1==consumer.m_tokens.size()); + currentEvent_->setConsumer(&consumer); + Handle nonesuch; + CPPUNIT_ASSERT(!(nonesuch=currentEvent_->getHandle(consumer.m_tokens[0]))); + CPPUNIT_ASSERT(!nonesuch.isValid()); + CPPUNIT_ASSERT(nonesuch.failedToGet()); + CPPUNIT_ASSERT_THROW(*nonesuch, cms::Exception); +} + +void testEvent::getFromEmpty() { + InputTag inputTag("moduleLabel", "instanceName"); + + IntConsumer consumer( std::vector{1,inputTag}); + consumer.updateLookup(InEvent, principal_->productLookup(),false); + assert(1==consumer.m_tokens.size()); + currentEvent_->setConsumer(&consumer); + CPPUNIT_ASSERT_THROW( (void) currentEvent_->get(consumer.m_tokens[0]), cms::Exception); +} + void testEvent::putAnIntProduct() { auto p = putProduct(std::make_unique(3),"int1",false); @@ -789,6 +821,153 @@ void testEvent::getByToken() { } +void testEvent::getHandle() { + typedef edmtest::IntProduct product_t; + typedef std::unique_ptr ap_t; + typedef Handle handle_t; + + ap_t one(new product_t(1)); + ap_t two(new product_t(2)); + ap_t three(new product_t(3)); + ap_t four(new product_t(4)); + addProduct(std::move(one), "int1_tag", "int1"); + addProduct(std::move(two), "int2_tag", "int2"); + addProduct(std::move(three), "int3_tag"); + addProduct(std::move(four), "nolabel_tag"); + + auto ap_vthing = std::make_unique>(); + addProduct(std::move(ap_vthing), "thing", ""); + + ap_t oneHundred(new product_t(100)); + addProduct(std::move(oneHundred), "int1_tag_late", "int1"); + + auto twoHundred = std::make_unique(200); + putProduct(std::move(twoHundred), "int1"); + + CPPUNIT_ASSERT(currentEvent_->size() == 7); + + IntProductConsumer consumer(std::vector { + InputTag("modMulti"), + InputTag("modMulti","int1"), + InputTag("modMulti", "nomatch"), + InputTag("modMulti", "int1", "EARLY"), + InputTag("modMulti", "int1", "LATE"), + InputTag("modMulti", "int1", "CURRENT"), + InputTag("modMulti", "int2", "EARLY"), + InputTag("modOne") + }); + consumer.updateLookup(InEvent, principal_->productLookup(),false); + + currentEvent_->setConsumer(&consumer); + + const auto modMultiToken = consumer.m_tokens[0]; + const auto modMultiInt1Token = consumer.m_tokens[1]; + const auto modMultinomatchToken = consumer.m_tokens[2]; + const auto modMultiInt1EarlyToken = consumer.m_tokens[3]; + const auto modMultiInt1LateToken = consumer.m_tokens[4]; + const auto modMultiInt1CurrentToken = consumer.m_tokens[5]; + const auto modMultiInt2EarlyToken = consumer.m_tokens[6]; + const auto modOneToken = consumer.m_tokens[7]; + + handle_t h; + CPPUNIT_ASSERT(h=currentEvent_->getHandle(modMultiToken)); + CPPUNIT_ASSERT(h->value == 3); + + CPPUNIT_ASSERT(h=currentEvent_->getHandle(modMultiInt1Token)); + CPPUNIT_ASSERT(h->value == 200); + + CPPUNIT_ASSERT(!(h=currentEvent_->getHandle(modMultinomatchToken))); + CPPUNIT_ASSERT(!h.isValid()); + CPPUNIT_ASSERT_THROW(*h, cms::Exception); + + CPPUNIT_ASSERT(h=currentEvent_->getHandle(modMultiInt1Token)); + CPPUNIT_ASSERT(h->value == 200); + + CPPUNIT_ASSERT(h=currentEvent_->getHandle(modMultiInt1EarlyToken)); + CPPUNIT_ASSERT(h->value == 1); + + CPPUNIT_ASSERT(h=currentEvent_->getHandle(modMultiInt1LateToken)); + CPPUNIT_ASSERT(h->value == 100); + + CPPUNIT_ASSERT(h=currentEvent_->getHandle(modMultiInt1CurrentToken)); + CPPUNIT_ASSERT(h->value == 200); + + CPPUNIT_ASSERT(h=currentEvent_->getHandle(modMultiInt2EarlyToken)); + CPPUNIT_ASSERT(h->value == 2); + + CPPUNIT_ASSERT(h=currentEvent_->getHandle(modOneToken)); + CPPUNIT_ASSERT(h->value == 4); + +} + +void testEvent::get_product() { + typedef edmtest::IntProduct product_t; + typedef std::unique_ptr ap_t; + typedef Handle handle_t; + + ap_t one(new product_t(1)); + ap_t two(new product_t(2)); + ap_t three(new product_t(3)); + ap_t four(new product_t(4)); + addProduct(std::move(one), "int1_tag", "int1"); + addProduct(std::move(two), "int2_tag", "int2"); + addProduct(std::move(three), "int3_tag"); + addProduct(std::move(four), "nolabel_tag"); + + auto ap_vthing = std::make_unique>(); + addProduct(std::move(ap_vthing), "thing", ""); + + ap_t oneHundred(new product_t(100)); + addProduct(std::move(oneHundred), "int1_tag_late", "int1"); + + auto twoHundred = std::make_unique(200); + putProduct(std::move(twoHundred), "int1"); + + CPPUNIT_ASSERT(currentEvent_->size() == 7); + + IntProductConsumer consumer(std::vector { + InputTag("modMulti"), + InputTag("modMulti","int1"), + InputTag("modMulti", "nomatch"), + InputTag("modMulti", "int1", "EARLY"), + InputTag("modMulti", "int1", "LATE"), + InputTag("modMulti", "int1", "CURRENT"), + InputTag("modMulti", "int2", "EARLY"), + InputTag("modOne") + }); + consumer.updateLookup(InEvent, principal_->productLookup(),false); + + currentEvent_->setConsumer(&consumer); + + const auto modMultiToken = consumer.m_tokens[0]; + const auto modMultiInt1Token = consumer.m_tokens[1]; + const auto modMultinomatchToken = consumer.m_tokens[2]; + const auto modMultiInt1EarlyToken = consumer.m_tokens[3]; + const auto modMultiInt1LateToken = consumer.m_tokens[4]; + const auto modMultiInt1CurrentToken = consumer.m_tokens[5]; + const auto modMultiInt2EarlyToken = consumer.m_tokens[6]; + const auto modOneToken = consumer.m_tokens[7]; + + CPPUNIT_ASSERT(currentEvent_->get(modMultiToken).value==3); + + CPPUNIT_ASSERT(currentEvent_->get(modMultiInt1Token).value == 200); + + CPPUNIT_ASSERT_THROW(currentEvent_->get(modMultinomatchToken), cms::Exception); + + CPPUNIT_ASSERT(currentEvent_->get(modMultiInt1Token).value == 200); + + CPPUNIT_ASSERT(currentEvent_->get(modMultiInt1EarlyToken).value == 1); + + CPPUNIT_ASSERT(currentEvent_->get(modMultiInt1LateToken).value == 100); + + CPPUNIT_ASSERT(currentEvent_->get(modMultiInt1CurrentToken).value == 200); + + CPPUNIT_ASSERT(currentEvent_->get(modMultiInt2EarlyToken).value == 2); + + CPPUNIT_ASSERT(currentEvent_->get(modOneToken).value == 4); + +} + void testEvent::getManyByType() { typedef edmtest::IntProduct product_t; typedef std::unique_ptr ap_t; From 0b155437f292fda3b55442c3e3424c9dda9b263c Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Sat, 19 Jan 2019 08:56:20 -0600 Subject: [PATCH 4/6] Switched to using Event::get and Event::getHandle --- FWCore/Framework/interface/GetterOfProducts.h | 11 +--- .../interface/stream/ThinningProducer.h | 3 +- .../test/stubs/RunLumiEventAnalyzer.cc | 4 +- .../Framework/test/stubs/TestGetPathStatus.cc | 12 ++-- FWCore/Framework/test/stubs/ToyAnalyzers.cc | 3 +- .../Framework/test/stubs/ToyIntProducers.cc | 9 +-- .../Framework/test/stubs/ToyRefProducers.cc | 12 ++-- FWCore/Integration/test/AcquireIntFilter.cc | 8 +-- FWCore/Integration/test/AcquireIntProducer.cc | 8 +-- .../test/AcquireIntStreamFilter.cc | 8 +-- .../test/AcquireIntStreamProducer.cc | 8 +-- .../test/AssociationMapAnalyzer.cc | 57 ++++++------------- .../test/AssociationMapProducer.cc | 22 +++---- .../Integration/test/OtherThingRefComparer.cc | 16 +++--- FWCore/Integration/test/TableTestModules.cc | 7 +-- FWCore/Integration/test/TestParentage.cc | 3 +- FWCore/Integration/test/ThingAnalyzer.cc | 57 ++++++------------- .../Integration/test/ThinningTestAnalyzer.cc | 12 ++-- .../Integration/test/ThinningThingProducer.cc | 4 +- .../Integration/test/TrackOfThingsProducer.cc | 3 +- .../test/WaitingThreadIntProducer.cc | 4 +- FWCore/Modules/src/LogErrorFilter.cc | 6 +- FWCore/Modules/src/PathStatusFilter.cc | 4 +- FWCore/Modules/src/TimeStudyModules.cc | 3 +- .../mkTemplates/EDAnalyzer/EDAnalyzer.cc | 17 ++---- 25 files changed, 94 insertions(+), 207 deletions(-) diff --git a/FWCore/Framework/interface/GetterOfProducts.h b/FWCore/Framework/interface/GetterOfProducts.h index 1d44eedbd31e0..082a6714c5a98 100644 --- a/FWCore/Framework/interface/GetterOfProducts.h +++ b/FWCore/Framework/interface/GetterOfProducts.h @@ -149,8 +149,7 @@ namespace edm { handles.reserve(tokens_->size()); edm::Handle handle; for (auto const& token : *tokens_) { - event.getByToken(token, handle); - if (handle.isValid()) { + if (auto handle = event.getHandle(token)) { handles.push_back(handle); } } @@ -161,10 +160,8 @@ namespace edm { handles.clear(); if(branchType_ == edm::InLumi ) { handles.reserve(tokens_->size()); - edm::Handle handle; for (auto const& token : *tokens_) { - lumi.getByToken(token, handle); - if (handle.isValid()) { + if (auto handle = lumi.getHandle(token)) { handles.push_back(handle); } } @@ -175,10 +172,8 @@ namespace edm { handles.clear(); if(branchType_ == edm::InRun) { handles.reserve(tokens_->size()); - edm::Handle handle; for (auto const& token : *tokens_) { - run.getByToken(token, handle); - if (handle.isValid()) { + if (auto handle = run.getHandle(token)) { handles.push_back(handle); } } diff --git a/FWCore/Framework/interface/stream/ThinningProducer.h b/FWCore/Framework/interface/stream/ThinningProducer.h index 59c44307cc3bc..381333b1468f0 100644 --- a/FWCore/Framework/interface/stream/ThinningProducer.h +++ b/FWCore/Framework/interface/stream/ThinningProducer.h @@ -76,8 +76,7 @@ namespace edm { void ThinningProducer:: produce(Event& event, EventSetup const& eventSetup) { - edm::Handle inputCollection; - event.getByToken(inputToken_, inputCollection); + auto inputCollection = event.getHandle(inputToken_); edm::Event const& constEvent = event; selector_->preChoose(inputCollection, constEvent, eventSetup); diff --git a/FWCore/Framework/test/stubs/RunLumiEventAnalyzer.cc b/FWCore/Framework/test/stubs/RunLumiEventAnalyzer.cc index 43dc7507ea833..6c62ca253963a 100644 --- a/FWCore/Framework/test/stubs/RunLumiEventAnalyzer.cc +++ b/FWCore/Framework/test/stubs/RunLumiEventAnalyzer.cc @@ -55,9 +55,7 @@ namespace edmtest { } if(dumpTriggerResults_) { - edm::Handle triggerResults; - event.getByToken(triggerResultsToken_, triggerResults); - if(triggerResults.isValid()) { + if( auto triggerResults=event.getHandle(triggerResultsToken_)) { edm::LogAbsolute("RunLumiEvent") << "TestFailuresAnalyzer dumping TriggerResults"; edm::LogAbsolute("RunLumiEvent") << *triggerResults; } else { diff --git a/FWCore/Framework/test/stubs/TestGetPathStatus.cc b/FWCore/Framework/test/stubs/TestGetPathStatus.cc index 264b38ae2c6e0..149f1795a5e21 100644 --- a/FWCore/Framework/test/stubs/TestGetPathStatus.cc +++ b/FWCore/Framework/test/stubs/TestGetPathStatus.cc @@ -44,24 +44,20 @@ namespace edmtest { void TestGetPathStatus::analyze(edm::StreamID, edm::Event const& event, edm::EventSetup const&) const { - edm::Handle hPathStatus; - event.getByToken(tokenPathStatus_, hPathStatus); - *hPathStatus; + auto const& pathStatus = event.get(tokenPathStatus_); unsigned int eventID = event.id().event(); - if (eventID < expectedStates_.size() && expectedStates_[eventID] != static_cast(hPathStatus->state())) { + if (eventID < expectedStates_.size() && expectedStates_[eventID] != static_cast(pathStatus.state())) { std::cerr << "TestGetPathStatus::analyze unexpected path status state" << std::endl; abort(); } if (eventID < expectedIndexes_.size() && - expectedIndexes_[eventID] != hPathStatus->index()) { + expectedIndexes_[eventID] != pathStatus.index()) { std::cerr << "TestGetPathStatus::analyze unexpected path status index " << std::endl; abort(); } - edm::Handle hEndPathStatus; - event.getByToken(tokenEndPathStatus_, hEndPathStatus); - *hEndPathStatus; + (void) event.get(tokenEndPathStatus_); } } using edmtest::TestGetPathStatus; diff --git a/FWCore/Framework/test/stubs/ToyAnalyzers.cc b/FWCore/Framework/test/stubs/ToyAnalyzers.cc index 5d5226013efff..bfacbccc31a1a 100644 --- a/FWCore/Framework/test/stubs/ToyAnalyzers.cc +++ b/FWCore/Framework/test/stubs/ToyAnalyzers.cc @@ -84,9 +84,8 @@ namespace edmtest { } void analyze(edm::StreamID, edm::Event const& iEvent, edm::EventSetup const&) const override { - edm::Handle h; for(auto const& token: m_tokens) { - iEvent.getByToken(token,h); + (void) iEvent.getHandle(token); } } diff --git a/FWCore/Framework/test/stubs/ToyIntProducers.cc b/FWCore/Framework/test/stubs/ToyIntProducers.cc index f1420c84b89d9..d2a7d3c522f3b 100644 --- a/FWCore/Framework/test/stubs/ToyIntProducers.cc +++ b/FWCore/Framework/test/stubs/ToyIntProducers.cc @@ -346,9 +346,8 @@ namespace edmtest { void IntProducerFromTransient::produce(edm::StreamID, edm::Event& e, edm::EventSetup const&) const { // EventSetup is not used. - edm::Handle result; - bool ok = e.getByToken(getToken_, result); - assert(ok); + auto result = e.getHandle(getToken_); + assert(result); e.emplace(putToken_,result.product()->value); } @@ -405,9 +404,7 @@ namespace edmtest { if (onlyGetOnEvent_ == 0u || e.eventAuxiliary().event() == onlyGetOnEvent_) { for(auto const& token: tokens_) { - edm::Handle anInt; - e.getByToken(token, anInt); - value += anInt->value; + value += e.get(token).value; } } e.emplace(putToken_,value); diff --git a/FWCore/Framework/test/stubs/ToyRefProducers.cc b/FWCore/Framework/test/stubs/ToyRefProducers.cc index ec2bd9eb32d85..2e1d17263e040 100644 --- a/FWCore/Framework/test/stubs/ToyRefProducers.cc +++ b/FWCore/Framework/test/stubs/ToyRefProducers.cc @@ -64,8 +64,7 @@ namespace edmtest { IntVecRefVectorProducer::produce(edm::StreamID, edm::Event& e, edm::EventSetup const&) const { // EventSetup is not used. // Get our input: - edm::Handle > input; - e.getByToken(target_, input); + edm::Handle > input = e.getHandle(target_); assert(input.isValid()); auto prod = std::make_unique(); @@ -102,8 +101,7 @@ namespace edmtest { IntVecRefToBaseVectorProducer::produce(edm::StreamID, edm::Event& e, edm::EventSetup const&) const { // EventSetup is not used. // Get our input: - edm::Handle > input; - e.getByToken(target_, input); + edm::Handle > input = e.getHandle(target_); assert(input.isValid()); edm::RefToBaseVector refVector; @@ -137,8 +135,7 @@ namespace edmtest { IntVecPtrVectorProducer::produce(edm::StreamID, edm::Event& e, edm::EventSetup const&) const { // EventSetup is not used. // Get our input: - edm::Handle > input; - e.getByToken(target_, input); + edm::Handle > input = e.getHandle(target_); assert(input.isValid()); auto prod = std::make_unique(); @@ -173,8 +170,7 @@ namespace edmtest { IntVecStdVectorPtrProducer::produce(edm::StreamID, edm::Event& e, edm::EventSetup const&) const { // EventSetup is not used. // Get our input: - edm::Handle > input; - e.getByToken(target_, input); + edm::Handle > input = e.getHandle(target_); assert(input.isValid()); auto prod = std::make_unique(); diff --git a/FWCore/Integration/test/AcquireIntFilter.cc b/FWCore/Integration/test/AcquireIntFilter.cc index 0b01188d2aee5..46e384cdf3c00 100644 --- a/FWCore/Integration/test/AcquireIntFilter.cc +++ b/FWCore/Integration/test/AcquireIntFilter.cc @@ -85,9 +85,7 @@ namespace edmtest { streamCacheData->processed().clear(); for(auto const& token: m_tokens) { - edm::Handle handle; - event.getByToken(token, handle); - streamCacheData->retrieved().push_back(handle->value); + streamCacheData->retrieved().push_back(event.get(token).value); } m_server->requestValuesAsync(streamID, &streamCacheData->retrieved(), &streamCacheData->processed(), holder); @@ -103,9 +101,7 @@ namespace edmtest { event.put(std::make_unique(sum)); // This part is here only for the Parentage test. - edm::Handle handle; - event.getByToken(m_tokenForProduce, handle); - *handle; + (void) event.get(m_tokenForProduce); return true; } diff --git a/FWCore/Integration/test/AcquireIntProducer.cc b/FWCore/Integration/test/AcquireIntProducer.cc index c7dfeb6c1461f..50a5b7678ebbc 100644 --- a/FWCore/Integration/test/AcquireIntProducer.cc +++ b/FWCore/Integration/test/AcquireIntProducer.cc @@ -87,9 +87,7 @@ namespace edmtest { streamCacheData->processed().clear(); for(auto const& token: m_tokens) { - edm::Handle handle; - event.getByToken(token, handle); - streamCacheData->retrieved().push_back(handle->value); + streamCacheData->retrieved().push_back(event.get(token).value); } m_server->requestValuesAsync(streamID.value(), &streamCacheData->retrieved(), @@ -109,9 +107,7 @@ namespace edmtest { event.put(std::make_unique(sum)); // This part is here only for the Parentage test. - edm::Handle handle; - event.getByToken(m_tokenForProduce, handle); - *handle; + (void) event.get(m_tokenForProduce); } void AcquireIntProducer::endJob() { diff --git a/FWCore/Integration/test/AcquireIntStreamFilter.cc b/FWCore/Integration/test/AcquireIntStreamFilter.cc index 840b50b01f49b..208e3595ba1c8 100644 --- a/FWCore/Integration/test/AcquireIntStreamFilter.cc +++ b/FWCore/Integration/test/AcquireIntStreamFilter.cc @@ -55,9 +55,7 @@ namespace edmtest { cache->processed().clear(); for(auto const& token: m_getTokens) { - edm::Handle handle; - event.getByToken(token, handle); - cache->retrieved().push_back(handle->value); + cache->retrieved().push_back(event.get(token).value); } edm::Service()->requestValuesAsync(m_token, @@ -76,9 +74,7 @@ namespace edmtest { event.put(std::make_unique(sum)); // This part is here only for the Parentage test. - edm::Handle handle; - event.getByToken(m_tokenForProduce, handle); - *handle; + (void) event.get(m_tokenForProduce); return true; } diff --git a/FWCore/Integration/test/AcquireIntStreamProducer.cc b/FWCore/Integration/test/AcquireIntStreamProducer.cc index 2bdc0f1fa917a..f0725f6a2ac7a 100644 --- a/FWCore/Integration/test/AcquireIntStreamProducer.cc +++ b/FWCore/Integration/test/AcquireIntStreamProducer.cc @@ -55,9 +55,7 @@ namespace edmtest { cache->processed().clear(); for(auto const& token: m_getTokens) { - edm::Handle handle; - event.getByToken(token, handle); - cache->retrieved().push_back(handle->value); + cache->retrieved().push_back(event.get(token).value); } edm::Service()->requestValuesAsync(m_token, @@ -76,9 +74,7 @@ namespace edmtest { event.put(std::make_unique(sum)); // This part is here only for the Parentage test. - edm::Handle handle; - event.getByToken(m_tokenForProduce, handle); - *handle; + (void) event.get(m_tokenForProduce); } } using edmtest::AcquireIntStreamProducer; diff --git a/FWCore/Integration/test/AssociationMapAnalyzer.cc b/FWCore/Integration/test/AssociationMapAnalyzer.cc index 4b07b87cdd002..e9f103fd51eb1 100644 --- a/FWCore/Integration/test/AssociationMapAnalyzer.cc +++ b/FWCore/Integration/test/AssociationMapAnalyzer.cc @@ -71,20 +71,16 @@ namespace edmtest { edm::Event const& event, edm::EventSetup const&) const { - edm::Handle > inputCollection1; - event.getByToken(inputToken1_, inputCollection1); + edm::Handle > inputCollection1 = event.getHandle(inputToken1_); - edm::Handle > inputCollection2; - event.getByToken(inputToken2_, inputCollection2); + edm::Handle > inputCollection2 = event.getHandle(inputToken2_); // Readout some entries from some AssociationMaps and check that // we readout the same content as was was put in. We know the values // by looking at the hard coded values in AssociationMapProducer. // The particular values used are arbitrary and have no meaning. - edm::Handle hAssociationMap1; - event.getByToken(associationMapToken1_, hAssociationMap1); - AssocOneToOne const& associationMap1 = *hAssociationMap1; + AssocOneToOne const& associationMap1 =event.get(associationMapToken1_); if(*associationMap1[edm::Ref >(inputCollection1, 0)] != 22 || *associationMap1[edm::Ref >(inputCollection1, 2)] != 24 || @@ -106,10 +102,7 @@ namespace edmtest { } // Case where handle arguments were used creating the AssociationMap - - edm::Handle hAssociationMap2; - event.getByToken(associationMapToken2_, hAssociationMap2); - AssocOneToOne const& associationMap2 = *hAssociationMap2; + AssocOneToOne const& associationMap2 = event.get(associationMapToken2_); if(*associationMap2[edm::Ref >(inputCollection1, 0)] != 22 || *associationMap2[edm::Ref >(inputCollection1, 2)] != 25 || @@ -126,9 +119,7 @@ namespace edmtest { // One to Value case - edm::Handle hAssociationMap3; - event.getByToken(associationMapToken3_, hAssociationMap3); - AssocOneToValue const& associationMap3 = *hAssociationMap3; + AssocOneToValue const& associationMap3 = event.get(associationMapToken3_); if(associationMap3[edm::Ref >(inputCollection1, 0)] != 11.0 || associationMap3[edm::Ref >(inputCollection1, 2)] != 12.0 || @@ -144,9 +135,7 @@ namespace edmtest { // One to Value case with handle argument - edm::Handle hAssociationMap4; - event.getByToken(associationMapToken4_, hAssociationMap4); - AssocOneToValue const& associationMap4 = *hAssociationMap4; + AssocOneToValue const& associationMap4 =event.get(associationMapToken4_); if(associationMap4[edm::Ref >(inputCollection1, 0)] != 21.0 || associationMap4[edm::Ref >(inputCollection1, 2)] != 22.0 || @@ -162,9 +151,7 @@ namespace edmtest { // One to Many - edm::Handle hAssociationMap5; - event.getByToken(associationMapToken5_, hAssociationMap5); - AssocOneToMany const& associationMap5 = *hAssociationMap5; + AssocOneToMany const& associationMap5 = event.get(associationMapToken5_); if(*associationMap5[edm::Ref >(inputCollection1, 0)].at(0) != 22 || *associationMap5[edm::Ref >(inputCollection1, 2)].at(1) != 27 || @@ -180,9 +167,7 @@ namespace edmtest { // One to Many With Quality - edm::Handle hAssociationMap6; - event.getByToken(associationMapToken6_, hAssociationMap6); - AssocOneToManyWithQuality const& associationMap6 = *hAssociationMap6; + AssocOneToManyWithQuality const& associationMap6 =event.get(associationMapToken6_); if(*associationMap6[edm::Ref >(inputCollection1, 0)].at(0).first != 22 || *associationMap6[edm::Ref >(inputCollection1, 2)].at(1).first != 25 || *associationMap6[edm::Ptr(inputCollection1, 0)].at(0).first != 22 || @@ -203,39 +188,33 @@ namespace edmtest { // One to One View - edm::Handle > inputView1; - event.getByToken(inputToken1V_, inputView1); + edm::View const& inputView1 = event.get(inputToken1V_); - edm::Handle > inputView2; - event.getByToken(inputToken2V_, inputView2); + edm::Handle> inputView2 = event.getHandle(inputToken2V_); - edm::Handle hAssociationMap7; - event.getByToken(associationMapToken7_, hAssociationMap7); - AssocOneToOneView const& associationMap7 = *hAssociationMap7; - if(*associationMap7[inputView1->refAt(0)] != 24 || - *associationMap7[inputView1->refAt(2)] != 25) { + AssocOneToOneView const& associationMap7 =event.get(associationMapToken7_); + if(*associationMap7[inputView1.refAt(0)] != 24 || + *associationMap7[inputView1.refAt(2)] != 25) { throw cms::Exception("TestFailure") << "unexpected result after using AssociationMap 15"; } AssocOneToOneView::const_iterator iter7 = associationMap7.begin(); ++iter7; if(*iter7->val != 25 || - iter7->key != inputView1->refAt(2)) { + iter7->key != inputView1.refAt(2)) { throw cms::Exception("TestFailure") << "unexpected result after using AssociationMap 16"; } // One to One View built with 2 arguments constructor - edm::Handle hAssociationMap8; - event.getByToken(associationMapToken8_, hAssociationMap8); - AssocOneToOneView const& associationMap8 = *hAssociationMap8; - if(*associationMap8[inputView1->refAt(0)] != 26 || - *associationMap8[inputView1->refAt(2)] != 27) { + AssocOneToOneView const& associationMap8 = event.get(associationMapToken8_); + if(*associationMap8[inputView1.refAt(0)] != 26 || + *associationMap8[inputView1.refAt(2)] != 27) { throw cms::Exception("TestFailure") << "unexpected result after using AssociationMap 17"; } AssocOneToOneView::const_iterator iter8 = associationMap8.begin(); ++iter8; if(*iter8->val != 27 || - iter8->key != inputView1->refAt(2)) { + iter8->key != inputView1.refAt(2)) { throw cms::Exception("TestFailure") << "unexpected result after using AssociationMap 18"; } } diff --git a/FWCore/Integration/test/AssociationMapProducer.cc b/FWCore/Integration/test/AssociationMapProducer.cc index 8d7846e310840..88be4bf5ae740 100644 --- a/FWCore/Integration/test/AssociationMapProducer.cc +++ b/FWCore/Integration/test/AssociationMapProducer.cc @@ -71,11 +71,9 @@ namespace edmtest { void AssociationMapProducer::produce(edm::Event& event, edm::EventSetup const&) { - edm::Handle > inputCollection1; - event.getByToken(inputToken1_, inputCollection1); + edm::Handle > inputCollection1 = event.getHandle(inputToken1_); - edm::Handle > inputCollection2; - event.getByToken(inputToken2_, inputCollection2); + edm::Handle > inputCollection2 = event.getHandle(inputToken2_); // insert some entries into some AssociationMaps, in another // module we will readout the contents and check that we readout @@ -124,22 +122,20 @@ namespace edmtest { AssocOneToManyWithQuality::data_type(edm::Ref >(inputCollection2, 7), 33.0)); event.put(std::move(assoc6)); - edm::Handle > inputView1; - event.getByToken(inputToken1V_, inputView1); + edm::View const& inputView1 = event.get(inputToken1V_); - edm::Handle > inputView2; - event.getByToken(inputToken2V_, inputView2); + edm::Handle > inputView2 = event.getHandle(inputToken2V_); auto assoc7 = std::make_unique(&event.productGetter()); - assoc7->insert(inputView1->refAt(0), inputView2->refAt(3)); - assoc7->insert(inputView1->refAt(2), inputView2->refAt(4)); + assoc7->insert(inputView1.refAt(0), inputView2->refAt(3)); + assoc7->insert(inputView1.refAt(2), inputView2->refAt(4)); event.put(std::move(assoc7)); - auto assoc8 = std::make_unique(edm::makeRefToBaseProdFrom(inputView1->refAt(0), event), + auto assoc8 = std::make_unique(edm::makeRefToBaseProdFrom(inputView1.refAt(0), event), edm::makeRefToBaseProdFrom(inputView2->refAt(0), event)); - assoc8->insert(inputView1->refAt(0), inputView2->refAt(5)); - assoc8->insert(inputView1->refAt(2), inputView2->refAt(6)); + assoc8->insert(inputView1.refAt(0), inputView2->refAt(5)); + assoc8->insert(inputView1.refAt(2), inputView2->refAt(6)); event.put(std::move(assoc8), "twoArg"); } } diff --git a/FWCore/Integration/test/OtherThingRefComparer.cc b/FWCore/Integration/test/OtherThingRefComparer.cc index 9e377a23d3fcc..b28388459ded4 100644 --- a/FWCore/Integration/test/OtherThingRefComparer.cc +++ b/FWCore/Integration/test/OtherThingRefComparer.cc @@ -34,16 +34,14 @@ namespace edmtest { } void OtherThingRefComparer::analyze(edm::Event const& e, edm::EventSetup const&) { - edm::Handle handle1_; - e.getByToken(token1_,handle1_); - edm::Handle handle2_; - e.getByToken(token2_,handle2_); + auto const& t1_ = e.get(token1_); + auto const& t2_ = e.get(token2_); - assert(handle1_->size() == handle2_->size()); + assert(t1_.size() == t2_.size()); { - auto iter2 = handle2_->begin(); - for(auto const& o1: *handle1_) { + auto iter2 = t2_.begin(); + for(auto const& o1: t1_) { if(o1.ref != iter2->ref) { throw cms::Exception("RefCompareFailure")<<"edm::Refs are not equal"<< o1.ref.id()<<" "<ref.id(); } @@ -52,8 +50,8 @@ namespace edmtest { } { - auto iter2 = handle2_->begin(); - for(auto const& o1: *handle1_) { + auto iter2 = t2_.begin(); + for(auto const& o1: t1_) { if(o1.ptr != iter2->ptr) { throw cms::Exception("RefCompareFailure")<<"edm::Ptrs are not equal"<ptr.id(); } diff --git a/FWCore/Integration/test/TableTestModules.cc b/FWCore/Integration/test/TableTestModules.cc index 9ebd2c8d19c60..667ee454dce05 100644 --- a/FWCore/Integration/test/TableTestModules.cc +++ b/FWCore/Integration/test/TableTestModules.cc @@ -53,16 +53,15 @@ namespace edmtest { } void analyze(edm::StreamID, edm::Event const& iEvent, edm::EventSetup const&) const final { - edm::Handle h; - iEvent.getByToken(tableToken_, h); + auto const& t = iEvent.get(tableToken_); - auto size = h->size(); + auto size = t.size(); if(size != anInts_.size()) { throw cms::Exception("RuntimeError")<<"Table size ("<() ) { throw cms::Exception("RuntimeError")<<"index "< h; - e.getByToken(token_, h); + edm::Handle h = e.getHandle(token_); edm::Provenance const* prov = h.provenance(); diff --git a/FWCore/Integration/test/ThingAnalyzer.cc b/FWCore/Integration/test/ThingAnalyzer.cc index 5d333d98bdbab..b9dd3a7bee312 100644 --- a/FWCore/Integration/test/ThingAnalyzer.cc +++ b/FWCore/Integration/test/ThingAnalyzer.cc @@ -77,32 +77,23 @@ namespace edmtest { auto const& run = lumi.getRun(); - edm::Handle h; - run.getByToken(beginRun_,h); - *h; + (void) run.get(beginRun_); - run.getByToken(endRun_,h); - shouldBeInvalid(h); + shouldBeInvalid(run.getHandle(endRun_)); - lumi.getByToken(beginLumi_,h); - *h; + (void) lumi.get(beginLumi_); - lumi.getByToken(endLumi_,h); - shouldBeInvalid(h); + shouldBeInvalid(lumi.getHandle(endLumi_)); - iEvent.getByToken(event_,h); - *h; + (void) iEvent.get(event_); } std::shared_ptr ThingAnalyzer::globalBeginRun(edm::Run const& iRun, edm::EventSetup const&) const { - edm::Handle h; - iRun.getByToken(beginRun_,h); - *h; + (void) iRun.get(beginRun_); - iRun.getByToken(endRun_,h); - shouldBeInvalid(h); + shouldBeInvalid(iRun.getHandle(endRun_)); return std::shared_ptr(); } @@ -110,12 +101,9 @@ namespace edmtest { void ThingAnalyzer::globalEndRun(edm::Run const& iRun, edm::EventSetup const&) const { - edm::Handle h; - iRun.getByToken(beginRun_,h); - *h; + (void) iRun.get(beginRun_); - iRun.getByToken(endRun_,h); - *h; + (void) iRun.get(endRun_); } std::shared_ptr @@ -123,19 +111,13 @@ namespace edmtest { { auto const& run = iLumi.getRun(); - edm::Handle h; - run.getByToken(beginRun_,h); - *h; + (void) run.get(beginRun_); - run.getByToken(endRun_,h); - shouldBeInvalid(h); + shouldBeInvalid(run.getHandle(endRun_)); - iLumi.getByToken(beginLumi_,h); - *h; + (void) iLumi.get(beginLumi_); - iLumi.getByToken(endLumi_,h); - shouldBeInvalid(h); - + shouldBeInvalid(iLumi.getHandle(endLumi_)); return std::shared_ptr(); } @@ -145,18 +127,13 @@ namespace edmtest { { auto const& run = iLumi.getRun(); - edm::Handle h; - run.getByToken(beginRun_,h); - *h; + (void) run.get(beginRun_); - run.getByToken(endRun_,h); - shouldBeInvalid(h); + shouldBeInvalid(run.getHandle(endRun_)); - iLumi.getByToken(beginLumi_,h); - *h; + (void) iLumi.get(beginLumi_); - iLumi.getByToken(endLumi_,h); - *h; + (void) iLumi.get(endLumi_); } diff --git a/FWCore/Integration/test/ThinningTestAnalyzer.cc b/FWCore/Integration/test/ThinningTestAnalyzer.cc index d8a6e98357c75..c8f3f24a02370 100644 --- a/FWCore/Integration/test/ThinningTestAnalyzer.cc +++ b/FWCore/Integration/test/ThinningTestAnalyzer.cc @@ -92,17 +92,13 @@ namespace edmtest { void ThinningTestAnalyzer::analyze(edm::Event const& event, edm::EventSetup const&) { - edm::Handle parentCollection; - event.getByToken(parentToken_, parentCollection); + edm::Handle parentCollection = event.getHandle(parentToken_); - edm::Handle thinnedCollection; - event.getByToken(thinnedToken_, thinnedCollection); + edm::Handle thinnedCollection = event.getHandle(thinnedToken_); - edm::Handle associationCollection; - event.getByToken(associationToken_, associationCollection); + edm::Handle associationCollection = event.getHandle(associationToken_); - edm::Handle trackCollection; - event.getByToken(trackToken_, trackCollection); + edm::Handle trackCollection = event.getHandle(trackToken_); unsigned int i = 0; if(parentWasDropped_) { diff --git a/FWCore/Integration/test/ThinningThingProducer.cc b/FWCore/Integration/test/ThinningThingProducer.cc index 9d119d30f6222..fc832d46d716c 100644 --- a/FWCore/Integration/test/ThinningThingProducer.cc +++ b/FWCore/Integration/test/ThinningThingProducer.cc @@ -57,9 +57,7 @@ namespace edmtest { } void ThinningThingSelector::preChoose(edm::Handle tc, edm::Event const& event, edm::EventSetup const& es) { - edm::Handle trackCollection; - event.getByToken(trackToken_, trackCollection); - for (auto const& track : *trackCollection) { + for (auto const& track : event.get(trackToken_)) { keysToSave_.insert(track.ref1.key() - offsetToThinnedKey_); keysToSave_.insert(track.ref2.key() - offsetToThinnedKey_); keysToSave_.insert(track.ptr1.key() - offsetToThinnedKey_); diff --git a/FWCore/Integration/test/TrackOfThingsProducer.cc b/FWCore/Integration/test/TrackOfThingsProducer.cc index d41345b54d870..edb72ba2e6bfe 100644 --- a/FWCore/Integration/test/TrackOfThingsProducer.cc +++ b/FWCore/Integration/test/TrackOfThingsProducer.cc @@ -51,8 +51,7 @@ namespace edmtest { void TrackOfThingsProducer::produce(edm::Event& event, edm::EventSetup const&) { - edm::Handle inputCollection; - event.getByToken(inputToken_, inputCollection); + edm::Handle inputCollection = event.getHandle(inputToken_); auto result = std::make_unique(); diff --git a/FWCore/Integration/test/WaitingThreadIntProducer.cc b/FWCore/Integration/test/WaitingThreadIntProducer.cc index c52aaf48de0d2..4d0be98223bc1 100644 --- a/FWCore/Integration/test/WaitingThreadIntProducer.cc +++ b/FWCore/Integration/test/WaitingThreadIntProducer.cc @@ -237,9 +237,7 @@ namespace edmtest { std::vector retrieved; for(auto const& token: m_tokens) { - edm::Handle handle; - e.getByToken(token, handle); - retrieved.push_back(handle->value); + retrieved.push_back(e.get(token).value); } std::vector values; diff --git a/FWCore/Modules/src/LogErrorFilter.cc b/FWCore/Modules/src/LogErrorFilter.cc index 3fc990a63b0a3..9c819218939aa 100644 --- a/FWCore/Modules/src/LogErrorFilter.cc +++ b/FWCore/Modules/src/LogErrorFilter.cc @@ -101,12 +101,8 @@ LogErrorFilter::~LogErrorFilter() { // ------------ method called on each new Event ------------ bool LogErrorFilter::filter(edm::Event& iEvent, edm::EventSetup const&) { - edm::Handle > errorsAndWarnings; - iEvent.getByToken(harvesterToken_,errorsAndWarnings); - if(errorsAndWarnings.failedToGet()) { - return false; - } else { + if(auto errorsAndWarnings = iEvent.getHandle(harvesterToken_)) { if (useThresholdsPerKind_){ unsigned int errorsBelowThreshold = 0; unsigned int warningsBelowThreshold = 0; diff --git a/FWCore/Modules/src/PathStatusFilter.cc b/FWCore/Modules/src/PathStatusFilter.cc index cbc04be34e17b..0b9c830299c5f 100644 --- a/FWCore/Modules/src/PathStatusFilter.cc +++ b/FWCore/Modules/src/PathStatusFilter.cc @@ -112,9 +112,7 @@ namespace edm { } bool evaluate(Event const& event) const override { - Handle handle; - event.getByToken(token_, handle); - return handle->accept(); + return event.get(token_).accept(); } private: diff --git a/FWCore/Modules/src/TimeStudyModules.cc b/FWCore/Modules/src/TimeStudyModules.cc index c141c575c77a6..9cb0f79c33aaa 100644 --- a/FWCore/Modules/src/TimeStudyModules.cc +++ b/FWCore/Modules/src/TimeStudyModules.cc @@ -58,9 +58,8 @@ namespace timestudy { void getAndSleep(edm::Event const& e) const { - edm::Handle h; for(auto const&t: tokens_) { - e.getByToken(t,h); + (void) e.getHandle(t); } //Event number minimum value is 1 usleep( eventTimes_[ (e.id().event()-1) % eventTimes_.size()]); diff --git a/FWCore/Skeletons/scripts/mkTemplates/EDAnalyzer/EDAnalyzer.cc b/FWCore/Skeletons/scripts/mkTemplates/EDAnalyzer/EDAnalyzer.cc index 6f2c56b48f3d4..6c86c97d5870c 100644 --- a/FWCore/Skeletons/scripts/mkTemplates/EDAnalyzer/EDAnalyzer.cc +++ b/FWCore/Skeletons/scripts/mkTemplates/EDAnalyzer/EDAnalyzer.cc @@ -107,20 +107,11 @@ __class__::analyze(const edm::Event& iEvent, const edm::EventSetup& iSetup) { using namespace edm; - Handle tracks; - iEvent.getByToken(tracksToken_, tracks); - for(TrackCollection::const_iterator itTrack = tracks->begin(); - itTrack != tracks->end(); - ++itTrack) { + for(const auto& track : iEvent.get(tracksToken_) ) { // do something with track parameters, e.g, plot the charge. - // int charge = itTrack->charge(); -@example_histo histo->Fill( itTrack->charge() ); - } - -#ifdef THIS_IS_AN_EVENT_EXAMPLE - Handle pIn; - iEvent.getByLabel("example",pIn); -#endif + // int charge = track.charge(); +@example_histo histo->Fill( track.charge() ); + } #ifdef THIS_IS_AN_EVENTSETUP_EXAMPLE ESHandle pSetup; From 6d61969b8eb9b9767829380b570e41cfea9e105d Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Sat, 19 Jan 2019 09:22:00 -0600 Subject: [PATCH 5/6] edm::get function now uses edm::Event::get call --- FWCore/Framework/interface/Event.h | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/FWCore/Framework/interface/Event.h b/FWCore/Framework/interface/Event.h index 7f5b33656d38f..76a1d08acf83f 100644 --- a/FWCore/Framework/interface/Event.h +++ b/FWCore/Framework/interface/Event.h @@ -740,7 +740,7 @@ namespace edm { // Will throw an exception if the collection is not available. template - T const& get(Event const& event, InputTag const& tag) { + T const& get(Event const& event, InputTag const& tag) noexcept(false) { Handle handle; event.getByLabel(tag, handle); // throw if the handle is not valid @@ -748,7 +748,7 @@ namespace edm { } template - T const& get(Event const& event, EDGetToken const& token) { + T const& get(Event const& event, EDGetToken const& token) noexcept(false) { Handle handle; event.getByToken(token, handle); // throw if the handle is not valid @@ -756,11 +756,8 @@ namespace edm { } template - T const& get(Event const& event, EDGetTokenT const& token) { - Handle handle; - event.getByToken(token, handle); - // throw if the handle is not valid - return * handle.product(); + T const& get(Event const& event, EDGetTokenT const& token) noexcept(false) { + return event.get(token); } } From 6d31f493c2fa305be811bad8217bd50888fbd75e Mon Sep 17 00:00:00 2001 From: Christopher Jones Date: Sat, 19 Jan 2019 14:05:34 -0600 Subject: [PATCH 6/6] Updated PileUpEventPrincipal to new edm::convert_handle call --- SimGeneral/MixingModule/interface/PileUpEventPrincipal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SimGeneral/MixingModule/interface/PileUpEventPrincipal.h b/SimGeneral/MixingModule/interface/PileUpEventPrincipal.h index 547aa96606695..94bb357f1ea7b 100644 --- a/SimGeneral/MixingModule/interface/PileUpEventPrincipal.h +++ b/SimGeneral/MixingModule/interface/PileUpEventPrincipal.h @@ -43,7 +43,7 @@ class PileUpEventPrincipal { bool getByLabel(edm::InputTag const& tag, edm::Handle& result) const { edm::BasicHandle bh = principal_.getByLabel(edm::PRODUCT_TYPE, edm::TypeID(typeid(T)), tag, nullptr, nullptr, mcc_); - convert_handle(std::move(bh), result); + result = edm::convert_handle(std::move(bh)); return result.isValid(); }