Merge pull request #22069 from makortel/replaceSwapWithMove

Replace swap_or_assign with move in edm::Wrapper<T> constructor
cms-sw · Feb 23, 2018 · 740adde · 740adde
2 parents 062dc64 + ef15111
commit 740adde
Show file tree

Hide file tree

Showing 21 changed files with 132 additions and 208 deletions.
diff --git a/DataFormats/Common/interface/DetSetVectorNew.h b/DataFormats/Common/interface/DetSetVectorNew.h
@@ -58,8 +58,27 @@ namespace edmNew {
       DetSetVectorTrans(): m_filling(false), m_dataSize(0){}
       DetSetVectorTrans& operator=(const DetSetVectorTrans&) = delete;
       DetSetVectorTrans(const DetSetVectorTrans&) = delete;
-      DetSetVectorTrans(DetSetVectorTrans&&) = default;
-      DetSetVectorTrans& operator=(DetSetVectorTrans&&) = default;
+      DetSetVectorTrans(DetSetVectorTrans&& rh) { // can't be default because of atomics
+        // better no one is filling...
+        assert(m_filling==false); assert(rh.m_filling==false);
+        m_getter = std::move(rh.m_getter);
+#ifdef DSVN_USE_ATOMIC
+        m_dataSize.store(rh.m_dataSize.exchange(m_dataSize.load()));
+#else
+        m_dataSize = std::move(rh.m_dataSize);
+#endif
+      }
+      DetSetVectorTrans& operator=(DetSetVectorTrans&& rh) {  // can't be default because of atomics
+        // better no one is filling...
+        assert(m_filling==false); assert(rh.m_filling==false);
+        m_getter = std::move(rh.m_getter);
+#ifdef DSVN_USE_ATOMIC
+        m_dataSize.store(rh.m_dataSize.exchange(m_dataSize.load()));
+#else
+        m_dataSize = std::move(rh.m_dataSize);
+#endif
+        return *this;
+      }
       mutable std::atomic<bool> m_filling;
       boost::any m_getter;
 #ifdef DSVN_USE_ATOMIC

diff --git a/DataFormats/Common/interface/Wrapper.h b/DataFormats/Common/interface/Wrapper.h
@@ -76,21 +76,13 @@ namespace edm {
     T obj;
   };
 
-  template<typename T>
-  inline
-  void swap_or_assign(T& a, T& b) {
-    detail::doSwapOrAssign<T>()(a, b);
-  } 
-
   template<typename T>
   Wrapper<T>::Wrapper(std::unique_ptr<T> ptr) :
     WrapperBase(),
     present(ptr.get() != nullptr),
     obj() {
     if (present) {
-      // The following will call swap if T has such a function,
-      // and use assignment if T has no such function.
-      swap_or_assign(obj, *ptr);
+      obj = std::move(*ptr);
     }
   }
 
@@ -101,9 +93,7 @@ namespace edm {
   obj() {
      std::unique_ptr<T> temp(ptr);
      if (present) {
-        // The following will call swap if T has such a function,
-        // and use assignment if T has no such function.
-        swap_or_assign(obj, *ptr);
+       obj = std::move(*ptr);
      }
   }
 

diff --git a/DataFormats/Common/interface/WrapperDetail.h b/DataFormats/Common/interface/WrapperDetail.h
@@ -20,29 +20,6 @@ namespace edm {
     using no_tag = std::false_type; // type indicating FALSE
     using yes_tag = std::true_type; // type indicating TRUE
 
-    // void swap_or_assign(T& a, T& b) will swap if T::swap(T&) is defined and assign otherwise
-    // Definitions for the following struct and function templates are not needed; we only require the declarations.
-    template<typename T, void (T::*)(T&)> struct swap_function;
-    template<typename T> static yes_tag has_swap(swap_function<T, &T::swap>* dummy);
-    template<typename T> static no_tag has_swap(...);
-
-    template<typename T>
-    struct has_swap_function {
-      static constexpr bool value = std::is_same<decltype(has_swap<T>(nullptr)), yes_tag>::value;
-    };
-
-    template<typename T, bool = has_swap_function<T>::value> struct doSwapOrAssign;
-    template<typename T> struct doSwapOrAssign<T, true> {
-      void operator()(T& thisProduct, T& otherProduct) {
-        thisProduct.swap(otherProduct);
-      }
-    };
-    template<typename T> struct doSwapOrAssign<T, false> {
-      void operator()(T& thisProduct, T& otherProduct) {
-        thisProduct = otherProduct;
-      }
-    };
-
     // valueTypeInfo_() will return typeid(T::value_type) if T::value_type is declared and typeid(void) otherwise.
     // Definitions for the following struct and function templates are not needed; we only require the declarations.
     template<typename T> static yes_tag has_value_type(typename T::value_type*);

diff --git a/DataFormats/Common/test/DetSetNew_t.cpp b/DataFormats/Common/test/DetSetNew_t.cpp
@@ -119,6 +119,12 @@ void TestDetSet::default_ctor() {
   CPPUNIT_ASSERT(detsets2.dataSize()==10);
   CPPUNIT_ASSERT(!detsets2.onDemand());
 
+  // test move
+  DSTV detsets3(4);
+  detsets = std::move(detsets3);
+  CPPUNIT_ASSERT(detsets.subdetId()==4);
+  CPPUNIT_ASSERT(detsets.size()==0);
+  CPPUNIT_ASSERT(detsets.dataSize()==0);
 }
 
 void TestDetSet::inserting() {
@@ -592,6 +598,9 @@ void TestDetSet::onDemand() {
   detsets2.swap(detsets);
   CPPUNIT_ASSERT(detsets2.onDemand());
 
+  DSTV detsets3;
+  detsets3 = std::move(detsets2);
+  CPPUNIT_ASSERT(detsets3.onDemand());
 }
 
 void TestDetSet::toRangeMap() {

diff --git a/DataFormats/Common/test/Wrapper_t.cpp b/DataFormats/Common/test/Wrapper_t.cpp
@@ -10,32 +10,33 @@
 
 #include "DataFormats/Common/interface/Wrapper.h"
 
-class CopyNoSwappy
+class CopyNoMove
 {
  public:
-  CopyNoSwappy() {}
-  CopyNoSwappy(CopyNoSwappy const&) { /* std::cout << "copied\n"; */ }
-  CopyNoSwappy& operator=(CopyNoSwappy const&) { /*std::cout << "assigned\n";*/ return *this;}
+  CopyNoMove() {}
+  CopyNoMove(CopyNoMove const&) { /* std::cout << "copied\n"; */ }
+  CopyNoMove& operator=(CopyNoMove const&) { /*std::cout << "assigned\n";*/ return *this;}
  private:
 };
 
-class SwappyNoCopy
+class MoveNoCopy
 {
  public:
-  SwappyNoCopy() {}
-  void swap(SwappyNoCopy&) { /* std::cout << "swapped\n";*/ }
+  MoveNoCopy() {}
+  MoveNoCopy(MoveNoCopy const&) = delete;
+  MoveNoCopy& operator=(MoveNoCopy const&) = delete;
+  MoveNoCopy(MoveNoCopy&&) { /* std::cout << "moved\n";*/ }
+  MoveNoCopy& operator=(MoveNoCopy&&) { /* std::cout << "moved\n";*/ return *this; }
  private:
-  SwappyNoCopy(SwappyNoCopy const&); // not implemented
-  SwappyNoCopy& operator=(SwappyNoCopy const&); // not implemented
 };
 
 void work()
 {
-  auto thing = std::make_unique<CopyNoSwappy>();
-  edm::Wrapper<CopyNoSwappy> wrap(std::move(thing));
+  auto thing = std::make_unique<CopyNoMove>();
+  edm::Wrapper<CopyNoMove> wrap(std::move(thing));
 
-  auto thing2 = std::make_unique<SwappyNoCopy>();
-  edm::Wrapper<SwappyNoCopy> wrap2(std::move(thing2));
+  auto thing2 = std::make_unique<MoveNoCopy>();
+  edm::Wrapper<MoveNoCopy> wrap2(std::move(thing2));
 
 
   auto thing3 = std::make_unique<std::vector<double>>(10,2.2);

diff --git a/JetMETCorrections/JetCorrector/interface/JetCorrector.h b/JetMETCorrections/JetCorrector/interface/JetCorrector.h
@@ -39,6 +39,8 @@ namespace reco {
     JetCorrector();
 #if !defined(__CINT__) && !defined(__MAKECINT__) && !defined(__REFLEX__)
     JetCorrector(std::unique_ptr<JetCorrectorImpl const> fImpl):impl_(std::move(fImpl)) {}
+    JetCorrector(JetCorrector&&) = default;
+    JetCorrector& operator=(JetCorrector&&) = default;
 
     typedef reco::Particle::LorentzVector LorentzVector;
 
@@ -79,15 +81,11 @@ namespace reco {
     // ---------- static member functions --------------------
 
     // ---------- member functions ---------------------------
-    void swap(JetCorrector& iOther) {
-      std::swap(impl_,iOther.impl_);
-    }
 
   private:
     JetCorrector(const JetCorrector&) = delete; 
 
     JetCorrector& operator=(const JetCorrector&) = delete; 
-    JetCorrector& operator=(JetCorrector&&) = default;
 
     // ---------- member data --------------------------------
     std::unique_ptr<JetCorrectorImpl const> impl_;

diff --git a/RecoPixelVertexing/PixelTriplets/interface/IntermediateHitTriplets.h b/RecoPixelVertexing/PixelTriplets/interface/IntermediateHitTriplets.h
@@ -259,15 +259,10 @@ class IntermediateHitTriplets {
   IntermediateHitTriplets(): seedingLayers_(nullptr) {}
   explicit IntermediateHitTriplets(const SeedingLayerSetsHits *seedingLayers): seedingLayers_(seedingLayers) {}
   IntermediateHitTriplets(const IntermediateHitTriplets& rh); // only to make ROOT dictionary generation happy
+  IntermediateHitTriplets(IntermediateHitTriplets&&) = default;
+  IntermediateHitTriplets& operator=(IntermediateHitTriplets&&) = default;
   ~IntermediateHitTriplets() = default;
 
-  void swap(IntermediateHitTriplets& rh) {
-    std::swap(seedingLayers_, rh.seedingLayers_);
-    std::swap(regions_, rh.regions_);
-    std::swap(layerTriplets_, rh.layerTriplets_);
-    std::swap(hitTriplets_, rh.hitTriplets_);
-  }
-
   void reserve(size_t nregions, size_t nlayersets, size_t ntriplets) {
     regions_.reserve(nregions);
     layerTriplets_.reserve(nregions*nlayersets);

diff --git a/RecoTracker/MeasurementDet/interface/MeasurementTrackerEvent.h b/RecoTracker/MeasurementDet/interface/MeasurementTrackerEvent.h
@@ -41,11 +41,8 @@ class MeasurementTrackerEvent {
 
    MeasurementTrackerEvent(const MeasurementTrackerEvent & other) = delete;
    MeasurementTrackerEvent & operator=(const MeasurementTrackerEvent & other) = delete;
-   MeasurementTrackerEvent(MeasurementTrackerEvent && other) = delete;
-   MeasurementTrackerEvent & operator=(MeasurementTrackerEvent && other) = delete;
-
-   //  for the edm wrapper...
-   void swap(MeasurementTrackerEvent &other) ;
+   MeasurementTrackerEvent(MeasurementTrackerEvent && other);
+   MeasurementTrackerEvent & operator=(MeasurementTrackerEvent && other);
 
 
    const MeasurementTracker & measurementTracker() const { return * theTracker; }
@@ -75,6 +72,4 @@ class MeasurementTrackerEvent {
    std::vector<bool> thePhase2OTClustersToSkip;
 };
 
-inline void swap(MeasurementTrackerEvent &a, MeasurementTrackerEvent &b) { a.swap(b); }
-
 #endif // MeasurementTrackerEvent_H
diff --git a/RecoTracker/MeasurementDet/src/MeasurementTrackerEvent.cc b/RecoTracker/MeasurementDet/src/MeasurementTrackerEvent.cc
@@ -10,19 +10,26 @@ MeasurementTrackerEvent::~MeasurementTrackerEvent() {
     }
 }
 
-void
-MeasurementTrackerEvent::swap(MeasurementTrackerEvent &other)
-{
-    if (&other != this) {
-        using std::swap;
-        swap(theTracker, other.theTracker);
-        swap(theStripData, other.theStripData);
-        swap(thePixelData, other.thePixelData);
-        swap(thePhase2OTData, other.thePhase2OTData);
-        swap(theOwner, other.theOwner);
-        swap(theStripClustersToSkip, other.theStripClustersToSkip);
-        swap(thePixelClustersToSkip, other.thePixelClustersToSkip);
-    }
+MeasurementTrackerEvent::MeasurementTrackerEvent(MeasurementTrackerEvent && other) {
+  theTracker = std::move(other.theTracker);
+  theStripData = std::move(other.theStripData);
+  thePixelData = std::move(other.thePixelData);
+  thePhase2OTData = std::move(other.thePhase2OTData);
+  theOwner = other.theOwner;
+  other.theOwner = false; // make sure to fully transfer the ownership
+  theStripClustersToSkip = std::move(other.theStripClustersToSkip);
+  thePixelClustersToSkip = std::move(other.thePixelClustersToSkip);
+}
+MeasurementTrackerEvent& MeasurementTrackerEvent::operator=(MeasurementTrackerEvent && other) {
+  theTracker = std::move(other.theTracker);
+  theStripData = std::move(other.theStripData);
+  thePixelData = std::move(other.thePixelData);
+  thePhase2OTData = std::move(other.thePhase2OTData);
+  theOwner = other.theOwner;
+  other.theOwner = false; // make sure to fully transfer the ownership
+  theStripClustersToSkip = std::move(other.theStripClustersToSkip);
+  thePixelClustersToSkip = std::move(other.thePixelClustersToSkip);
+  return *this;
 }
 
 MeasurementTrackerEvent::MeasurementTrackerEvent(const MeasurementTrackerEvent &trackerEvent,

diff --git a/RecoTracker/TkHitPairs/interface/IntermediateHitDoublets.h b/RecoTracker/TkHitPairs/interface/IntermediateHitDoublets.h
@@ -20,6 +20,8 @@ namespace ihd {
       layerSetBeginIndex_(ind),
       layerSetEndIndex_(ind)
     {}
+    RegionIndex(RegionIndex&&) = default;
+    RegionIndex& operator=(RegionIndex&&) = default;
 
     void setLayerSetsEnd(unsigned int end) { layerSetEndIndex_ = end; }
 
@@ -194,14 +196,10 @@ class IntermediateHitDoublets {
   IntermediateHitDoublets(): seedingLayers_(nullptr) {}
   explicit IntermediateHitDoublets(const SeedingLayerSetsHits *seedingLayers): seedingLayers_(seedingLayers) {}
   IntermediateHitDoublets(const IntermediateHitDoublets& rh); // only to make ROOT dictionary generation happy
+  IntermediateHitDoublets(IntermediateHitDoublets&&) = default;
+  IntermediateHitDoublets& operator=(IntermediateHitDoublets&&) = default;
   ~IntermediateHitDoublets() = default;
 
-  void swap(IntermediateHitDoublets& rh) {
-    std::swap(seedingLayers_, rh.seedingLayers_);
-    std::swap(regions_, rh.regions_);
-    std::swap(layerPairs_, rh.layerPairs_);
-  }
-
   void reserve(size_t nregions, size_t nlayersets) {
     regions_.reserve(nregions);
     layerPairs_.reserve(nregions*nlayersets);

diff --git a/RecoTracker/TkHitPairs/interface/LayerHitMapCache.h b/RecoTracker/TkHitPairs/interface/LayerHitMapCache.h
@@ -20,7 +20,10 @@ class LayerHitMapCache {
     using ValueType = RecHitsSortedInPhi;
     using KeyType = int;
     SimpleCache(unsigned int initSize) : theContainer(initSize){}
-    SimpleCache(SimpleCache&& rh): theContainer(std::move(rh.theContainer)) {}
+    SimpleCache(const SimpleCache&) = delete;
+    SimpleCache& operator=(const SimpleCache&) = delete;
+    SimpleCache(SimpleCache&&) = default;
+    SimpleCache& operator=(SimpleCache&&) = default;
     ~SimpleCache() { clear(); }
     void resize(int size) { theContainer.resize(size); }
     const ValueType*  get(KeyType key) const { return theContainer[key].get();}
@@ -45,15 +48,15 @@ class LayerHitMapCache {
     }
   private:
     std::vector<mayown_ptr<ValueType> > theContainer;
-  private:
-    SimpleCache(const SimpleCache &) { }
   };
 
 private:
   typedef SimpleCache Cache;
 public:
   LayerHitMapCache(unsigned int initSize=50) : theCache(initSize) { }
-  LayerHitMapCache(LayerHitMapCache&& rh): theCache(std::move(rh.theCache)) {}
+  LayerHitMapCache(LayerHitMapCache&&) = default;
+  LayerHitMapCache& operator=(LayerHitMapCache&&) = default;
+
 
   void clear() { theCache.clear(); }
 

diff --git a/RecoTracker/TkHitPairs/interface/RegionsSeedingHitSets.h b/RecoTracker/TkHitPairs/interface/RegionsSeedingHitSets.h
@@ -45,13 +45,12 @@ class RegionsSeedingHitSets {
 
   // constructors
   RegionsSeedingHitSets() = default;
+  RegionsSeedingHitSets(const RegionsSeedingHitSets&) = delete;
+  RegionsSeedingHitSets& operator=(const RegionsSeedingHitSets&) = delete;
+  RegionsSeedingHitSets(RegionsSeedingHitSets&&) = default;
+  RegionsSeedingHitSets& operator=(RegionsSeedingHitSets&&) = default;
   ~RegionsSeedingHitSets() = default;
 
-  void swap(RegionsSeedingHitSets& rh) {
-    regions_.swap(rh.regions_);
-    hitSets_.swap(rh.hitSets_);
-  }
-
   void reserve(size_t nregions, size_t nhitsets) {
     regions_.reserve(nregions);
     hitSets_.reserve(nhitsets);

diff --git a/SimDataFormats/Associations/interface/MuonToTrackingParticleAssociator.h b/SimDataFormats/Associations/interface/MuonToTrackingParticleAssociator.h
@@ -16,11 +16,13 @@ namespace reco {
 
   public:
 
-    MuonToTrackingParticleAssociator ();
-    ~MuonToTrackingParticleAssociator ();
+    MuonToTrackingParticleAssociator () = default;
+    ~MuonToTrackingParticleAssociator () = default;
 #ifndef __GCCXML__
     MuonToTrackingParticleAssociator(std::unique_ptr<MuonToTrackingParticleAssociatorBaseImpl>);
 #endif
+    MuonToTrackingParticleAssociator(MuonToTrackingParticleAssociator&&) = default;
+    MuonToTrackingParticleAssociator& operator=(MuonToTrackingParticleAssociator&&) = default;
 
     void associateMuons(MuonToSimCollection & recoToSim, SimToMuonCollection & simToReco,
                         const edm::RefToBaseVector<reco::Muon> & muons, MuonTrackType type,
@@ -33,15 +35,11 @@ namespace reco {
       impl_->associateMuons(recoToSim, simToReco, muons, type, tpColl);
     }
 
-    void swap(MuonToTrackingParticleAssociator& iOther) {
-      std::swap(impl_, iOther.impl_);
-    }
-
   private:
     MuonToTrackingParticleAssociator( const MuonToTrackingParticleAssociator&) = delete;
     MuonToTrackingParticleAssociator& operator=( const MuonToTrackingParticleAssociator&) = delete;
 
-    MuonToTrackingParticleAssociatorBaseImpl const* impl_;
+    std::unique_ptr<MuonToTrackingParticleAssociatorBaseImpl const> impl_;
   };
 }