Merge pull request #30506 from makortel/esgettokenThrowUninitialized

Throw a separate exception if ESGetToken is uninitialized
cms-sw · Sep 8, 2020 · 6ee16df · 6ee16df
2 parents 2ce72b2 + c62a716
commit 6ee16df
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 22 deletions.
diff --git a/FWCore/Framework/interface/EventSetupRecord.h b/FWCore/Framework/interface/EventSetupRecord.h
@@ -111,10 +111,9 @@ namespace edm {
 
       template <typename HolderT>
       bool get(char const* iName, HolderT& iHolder) const {
-        if
-          UNLIKELY(requireTokens_) {
-            throwCalledGetWithoutToken(heterocontainer::className<typename HolderT::value_type>(), iName);
-          }
+        if UNLIKELY (requireTokens_) {
+          throwCalledGetWithoutToken(heterocontainer::className<typename HolderT::value_type>(), iName);
+        }
         typename HolderT::value_type const* value = nullptr;
         ComponentDescription const* desc = nullptr;
         std::shared_ptr<ESHandleExceptionFactory> whyFailedFactory;
@@ -136,10 +135,9 @@ namespace edm {
 
       template <typename HolderT>
       bool get(ESInputTag const& iTag, HolderT& iHolder) const {
-        if
-          UNLIKELY(requireTokens_) {
-            throwCalledGetWithoutToken(heterocontainer::className<typename HolderT::value_type>(), iTag.data().c_str());
-          }
+        if UNLIKELY (requireTokens_) {
+          throwCalledGetWithoutToken(heterocontainer::className<typename HolderT::value_type>(), iTag.data().c_str());
+        }
         typename HolderT::value_type const* value = nullptr;
         ComponentDescription const* desc = nullptr;
         std::shared_ptr<ESHandleExceptionFactory> whyFailedFactory;
@@ -207,26 +205,32 @@ namespace edm {
     protected:
       template <template <typename> typename H, typename T, typename R>
       H<T> getHandleImpl(ESGetToken<T, R> const& iToken) const {
-        if
-          UNLIKELY(iToken.transitionID() != transitionID()) { throwWrongTransitionID(); }
-        assert(iToken.isInitialized());
+        if UNLIKELY (not iToken.isInitialized()) {
+          std::rethrow_exception(makeUninitializedTokenException(this->key(), DataKey::makeTypeTag<T>()));
+        }
+        if UNLIKELY (iToken.transitionID() != transitionID()) {
+          throwWrongTransitionID();
+        }
         assert(getTokenIndices_);
         //need to check token has valid index
-        if
-          UNLIKELY(not iToken.hasValidIndex()) { return invalidTokenHandle<H>(iToken); }
+        if UNLIKELY (not iToken.hasValidIndex()) {
+          return invalidTokenHandle<H>(iToken);
+        }
 
         auto proxyIndex = getTokenIndices_[iToken.index().value()];
-        if
-          UNLIKELY(proxyIndex.value() == std::numeric_limits<int>::max()) { return noProxyHandle<H>(iToken); }
+        if UNLIKELY (proxyIndex.value() == std::numeric_limits<int>::max()) {
+          return noProxyHandle<H>(iToken);
+        }
 
         T const* value = nullptr;
         ComponentDescription const* desc = nullptr;
         std::shared_ptr<ESHandleExceptionFactory> whyFailedFactory;
 
         impl_->getImplementation(value, proxyIndex, H<T>::transientAccessOnly, desc, whyFailedFactory, eventSetupImpl_);
 
-        if
-          UNLIKELY(not value) { return H<T>(std::move(whyFailedFactory)); }
+        if UNLIKELY (not value) {
+          return H<T>(std::move(whyFailedFactory));
+        }
         return H<T>(value, desc);
       }
 
@@ -251,8 +255,9 @@ namespace edm {
       template <template <typename> typename H, typename T, typename R>
       H<T> invalidTokenHandle(ESGetToken<T, R> const& iToken) const {
         auto const key = this->key();
-        return H<T>{
-            makeESHandleExceptionFactory([key] { return makeInvalidTokenException(key, DataKey::makeTypeTag<T>()); })};
+        return H<T>{makeESHandleExceptionFactory([key, transitionID = iToken.transitionID()] {
+          return makeInvalidTokenException(key, DataKey::makeTypeTag<T>(), transitionID);
+        })};
       }
 
       template <template <typename> typename H, typename T, typename R>
@@ -269,7 +274,8 @@ namespace edm {
                                ComponentDescription const*& iDesc,
                                bool iTransientAccessOnly) const;
 
-      static std::exception_ptr makeInvalidTokenException(EventSetupRecordKey const&, TypeTag const&);
+      static std::exception_ptr makeUninitializedTokenException(EventSetupRecordKey const&, TypeTag const&);
+      static std::exception_ptr makeInvalidTokenException(EventSetupRecordKey const&, TypeTag const&, unsigned int);
       void throwWrongTransitionID() const;
       static void throwCalledGetWithoutToken(const char* iTypeName, const char* iLabel);
       // ---------- member data --------------------------------

diff --git a/FWCore/Framework/src/EventSetupRecord.cc b/FWCore/Framework/src/EventSetupRecord.cc
@@ -70,8 +70,8 @@ namespace edm {
       iException.addContext(ost.str());
     }
 
-    std::exception_ptr EventSetupRecord::makeInvalidTokenException(EventSetupRecordKey const& iRecordKey,
-                                                                   TypeTag const& iDataKey) {
+    std::exception_ptr EventSetupRecord::makeUninitializedTokenException(EventSetupRecordKey const& iRecordKey,
+                                                                         TypeTag const& iDataKey) {
       cms::Exception ex("InvalidESGetToken");
       ex << "Attempted to get data using an invalid token of type ESGetToken<" << iDataKey.name() << ","
          << iRecordKey.name()
@@ -80,6 +80,17 @@ namespace edm {
       return std::make_exception_ptr(ex);
     }
 
+    std::exception_ptr EventSetupRecord::makeInvalidTokenException(EventSetupRecordKey const& iRecordKey,
+                                                                   TypeTag const& iDataKey,
+                                                                   unsigned int iTransitionID) {
+      cms::Exception ex("InvalidESGetToken");
+      ex << "Attempted to get data using an invalid token of type ESGetToken<" << iDataKey.name() << ","
+         << iRecordKey.name() << "> that had transition ID set (" << iTransitionID
+         << ") but not the index.\n"
+            "This should not happen, please contact core framework developers.";
+      return std::make_exception_ptr(ex);
+    }
+
     void EventSetupRecord::throwWrongTransitionID() const {
       cms::Exception ex("ESGetTokenWrongTransition");
       ex << "The transition ID stored in the ESGetToken does not match the\n"

diff --git a/FWCore/Framework/test/eventsetup_t.cppunit.cc b/FWCore/Framework/test/eventsetup_t.cppunit.cc
@@ -588,6 +588,7 @@ namespace {
     }
 
     ESGetToken<edm::eventsetup::test::DummyData, edm::DefaultRecord> m_token;
+    ESGetToken<edm::eventsetup::test::DummyData, edm::DefaultRecord> m_tokenUninitialized;
   };
 
   //This just tests that the constructs will properly compile
@@ -831,6 +832,14 @@ void testEventsetup::getDataWithESGetTokenTest() {
                             true};
       auto const& data = eventSetup.getData(consumer.m_token);
       CPPUNIT_ASSERT(kGood.value_ == data.value_);
+      bool uninitializedTokenThrewException = false;
+      try {
+        (void)eventSetup.getData(consumer.m_tokenUninitialized);
+      } catch (cms::Exception& ex) {
+        uninitializedTokenThrewException = true;
+        CPPUNIT_ASSERT(ex.category() == "InvalidESGetToken");
+      }
+      CPPUNIT_ASSERT(uninitializedTokenThrewException);
     }
 
     {