Merge pull request #31701 from Dr15Jones/fixEDGetTokenT

Avoid calling the wrong constructor for EDGetTokenT
cms-sw · Oct 9, 2020 · 011507b · 011507b
2 parents 9e1dfb0 + 6e41cd2
commit 011507b
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 55 deletions.
diff --git a/FWCore/Utilities/interface/EDGetToken.h b/FWCore/Utilities/interface/EDGetToken.h
@@ -88,7 +88,8 @@ namespace edm {
 
     //Needed to avoid EDGetTokenT(ADAPTER&&) from being called instead
     // when we can use C++20 concepts we can avoid the problem using a constraint
-    constexpr EDGetTokenT(EDGetTokenT& iOther) noexcept : m_value{iOther.m_value} {}
+    constexpr EDGetTokenT(EDGetTokenT<T>& iOther) noexcept : m_value{iOther.m_value} {}
+    constexpr EDGetTokenT(const EDGetTokenT<T>&& iOther) noexcept : m_value{iOther.m_value} {}
 
     constexpr EDGetTokenT& operator=(EDGetTokenT<T>& iOther) {
       return (*this = const_cast<const EDGetTokenT<T>&>(iOther));

diff --git a/FWCore/Utilities/interface/ESGetToken.h b/FWCore/Utilities/interface/ESGetToken.h
@@ -63,6 +63,10 @@ namespace edm {
     constexpr ESGetToken<ESProduct, ESRecord>& operator=(ESGetToken<ESProduct, ESRecord>& iOther) noexcept {
       return (*this = const_cast<ESGetToken<ESProduct, ESRecord> const&>(iOther));
     }
+    constexpr ESGetToken(ESGetToken<ESProduct, ESRecord> const&& iOther) : ESGetToken(iOther) {}
+    constexpr ESGetToken<ESProduct, ESRecord>& operator=(ESGetToken<ESProduct, ESRecord> const&& iOther) noexcept {
+      return (*this = iOther);
+    }
 
     constexpr unsigned int transitionID() const noexcept { return m_transitionID; }
     constexpr bool isInitialized() const noexcept { return transitionID() != std::numeric_limits<unsigned int>::max(); }

diff --git a/FWCore/Utilities/test/BuildFile.xml b/FWCore/Utilities/test/BuildFile.xml
@@ -54,9 +54,6 @@
   <use name="tbb"/>
 </bin>
 
-<bin file="EDGetToken_t.cpp">
-</bin>
-
 <bin file="RunningAverage_t.cpp">
   <use name="tbb"/>
 </bin>
diff --git a/FWCore/Utilities/test/EDGetToken_t.cpp b/FWCore/Utilities/test/EDGetToken_t.cpp
diff --git a/FWCore/Utilities/test/test_catch2_EDGetToken.cc b/FWCore/Utilities/test/test_catch2_EDGetToken.cc
@@ -0,0 +1,96 @@
+#include <functional>
+#include "FWCore/Utilities/interface/EDGetToken.h"
+#include "catch.hpp"
+
+namespace edm {
+  class TestEDGetToken {
+  public:
+    template <typename... Args>
+    static edm::EDGetToken makeToken(Args&&... iArgs) {
+      return edm::EDGetToken(std::forward<Args>(iArgs)...);
+    }
+
+    template <typename T, typename... Args>
+    static edm::EDGetTokenT<T> makeTokenT(Args&&... iArgs) {
+      return edm::EDGetTokenT<T>(std::forward<Args>(iArgs)...);
+    }
+
+    template <typename T, typename... Args>
+    static const edm::EDGetTokenT<T> makeConstTokenT(Args&&... iArgs) {
+      return edm::EDGetTokenT<T>(std::forward<Args>(iArgs)...);
+    }
+  };
+}  // namespace edm
+
+namespace {
+  class Adapter {
+  public:
+    template <typename T>
+    edm::EDGetTokenT<T> consumes() const {
+      return edm::TestEDGetToken::makeTokenT<T>(11U);
+    }
+  };
+}  // namespace
+
+TEST_CASE("Test EDGetToken", "[EDGetToken]") {
+  SECTION("EDGetTokenT") {
+    SECTION("No argument ctr") {
+      edm::EDGetTokenT<int> token1 = edm::TestEDGetToken::makeTokenT<int>();
+      REQUIRE(token1.isUninitialized());
+      REQUIRE((token1.index() == 0xFFFFFFFF));
+    }
+    SECTION("1 arg ctr") {
+      edm::EDGetTokenT<int> token2 = edm::TestEDGetToken::makeTokenT<int>(11U);
+      REQUIRE(!token2.isUninitialized());
+      REQUIRE((token2.index() == 11));
+    }
+    SECTION("non const arg copy ctr") {
+      edm::EDGetTokenT<int> token2 = edm::TestEDGetToken::makeTokenT<int>(11U);
+      edm::EDGetTokenT<int> token3(token2);
+      REQUIRE(!token3.isUninitialized());
+      REQUIRE((token3.index() == 11));
+    }
+
+    SECTION("const arg copy ctr") {
+      const edm::EDGetTokenT<int> cToken2 = edm::TestEDGetToken::makeTokenT<int>(11U);
+      edm::EDGetTokenT<int> token4(cToken2);
+      REQUIRE(!token4.isUninitialized());
+      REQUIRE(token4.index() == 11);
+    }
+
+    SECTION("const arg copy ctr with move") {
+      auto const t = edm::TestEDGetToken::makeConstTokenT<int>(11U);
+      const edm::EDGetTokenT<int> cToken2{std::move(t)};
+      REQUIRE(!cToken2.isUninitialized());
+      REQUIRE(cToken2.index() == 11);
+    }
+
+    SECTION("Use Adapter") {
+      Adapter a;
+      const edm::EDGetTokenT<int> cToken2{a};
+      REQUIRE(!cToken2.isUninitialized());
+      REQUIRE(cToken2.index() == 11);
+    }
+  }
+
+  SECTION("EDGetToken") {
+    SECTION("No arg ctr") {
+      edm::EDGetToken token10 = edm::TestEDGetToken::makeToken();
+      REQUIRE(token10.isUninitialized());
+      REQUIRE(token10.index() == 0xFFFFFFFF);
+    }
+
+    SECTION("1 arg ctr") {
+      edm::EDGetToken token11 = edm::TestEDGetToken::makeToken(100);
+      REQUIRE(!token11.isUninitialized());
+      REQUIRE(token11.index() == 100);
+    }
+
+    SECTION("EDGetTokenT to EDGetToken ctr") {
+      edm::EDGetTokenT<int> token2 = edm::TestEDGetToken::makeTokenT<int>(11U);
+      edm::EDGetToken token12(token2);
+      REQUIRE(!token12.isUninitialized());
+      REQUIRE(token12.index() == 11);
+    }
+  }
+}