Have TkCloner return std::unique_ptr for factory functions

The static analyzer was complaining about TkCloner returning
non-const pointers from functions. These functions were purely
factory methods so the complaints were not justified. However,
the best way to silence those complaints was to change the return
type to std::unique_ptr since this enforces the ownership transfer
which was merely implicit in the previous API.

DataFormats/TrackerRecHit2D/interface/ProjectedSiStripRecHit2D.h

@@ -74,7 +74,7 @@ class ProjectedSiStripRecHit2D GCC11_FINAL  : public TrackerSingleRecHit  {
 private:
   // double dispatch
   virtual ProjectedSiStripRecHit2D * clone(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const {
-    return cloner(*this,tsos);
+    return cloner(*this,tsos).release();
   }
 #ifdef NO_DICT
   virtual  ConstRecHitPointer cloneSH(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const {

DataFormats/TrackerRecHit2D/interface/SiPixelRecHit.h

@@ -58,7 +58,7 @@ class SiPixelRecHit GCC11_FINAL : public TrackerSingleRecHit {
 private:
   // double dispatch
   virtual SiPixelRecHit * clone(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const {
-    return cloner(*this,tsos);
+    return cloner(*this,tsos).release();
   }
 #ifdef NO_DICT
   virtual  RecHitPointer cloneSH(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const {

DataFormats/TrackerRecHit2D/interface/SiStripMatchedRecHit2D.h

@@ -64,7 +64,7 @@ class SiStripMatchedRecHit2D GCC11_FINAL : public BaseTrackerRecHit {
 private:
   // double dispatch
   virtual SiStripMatchedRecHit2D * clone(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const {
-    return cloner(*this,tsos);
+    return cloner(*this,tsos).release();
   }
 #ifdef NO_DICT
   virtual  RecHitPointer cloneSH(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const {

DataFormats/TrackerRecHit2D/interface/SiStripRecHit1D.h

@@ -42,7 +42,7 @@ class SiStripRecHit1D GCC11_FINAL : public TrackerSingleRecHit {
 private:
   // double dispatch
   virtual SiStripRecHit1D * clone(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const GCC11_OVERRIDE {
-    return cloner(*this,tsos);
+    return cloner(*this,tsos).release();
   }
 #ifdef NO_DICT
   virtual  RecHitPointer cloneSH(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const GCC11_OVERRIDE {

DataFormats/TrackerRecHit2D/interface/SiStripRecHit2D.h

@@ -40,8 +40,8 @@ class SiStripRecHit2D GCC11_FINAL : public TrackerSingleRecHit {
   virtual bool canImproveWithTrack() const GCC11_OVERRIDE {return true;}
 private:
   // double dispatch
-  virtual SiStripRecHit2D * clone(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const GCC11_OVERRIDE {
-    return cloner(*this,tsos);
+  virtual SiStripRecHit2D* clone(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const GCC11_OVERRIDE {
+    return cloner(*this,tsos).release();
   }
 #ifdef NO_DICT
   virtual  RecHitPointer cloneSH(TkCloner const& cloner, TrajectoryStateOnSurface const& tsos) const GCC11_OVERRIDE {

DataFormats/TrackerRecHit2D/interface/TkCloner.h

@@ -1,8 +1,10 @@
 #ifndef TKClonerRecHit_H
 #define TKClonerRecHit_H
 
+#include <memory>
 #include "DataFormats/TrackingRecHit/interface/TrackingRecHit.h"
 #include "FWCore/Utilities/interface/thread_safety_macros.h"
+#include "FWCore/Utilities/interface/HideStdUniquePtrFromRoot.h"
 
 class SiPixelRecHit;
 class SiStripRecHit2D;
@@ -23,11 +25,11 @@ class TkCloner {
   }
 #endif
 
-  virtual SiPixelRecHit * operator()(SiPixelRecHit const & hit, TrajectoryStateOnSurface const& tsos) const=0;
-  virtual SiStripRecHit2D * operator()(SiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const=0;
-  virtual SiStripRecHit1D * operator()(SiStripRecHit1D const & hit, TrajectoryStateOnSurface const& tsos) const=0;
-  virtual SiStripMatchedRecHit2D * operator()(SiStripMatchedRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const=0;
-  virtual ProjectedSiStripRecHit2D * operator()(ProjectedSiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const=0;
+  virtual std::unique_ptr<SiPixelRecHit> operator()(SiPixelRecHit const & hit, TrajectoryStateOnSurface const& tsos) const=0;
+  virtual std::unique_ptr<SiStripRecHit2D> operator()(SiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const=0;
+  virtual std::unique_ptr<SiStripRecHit1D> operator()(SiStripRecHit1D const & hit, TrajectoryStateOnSurface const& tsos) const=0;
+  virtual std::unique_ptr<SiStripMatchedRecHit2D> operator()(SiStripMatchedRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const=0;
+  virtual std::unique_ptr<ProjectedSiStripRecHit2D> operator()(ProjectedSiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const=0;
 
 #ifdef NO_DICT
   virtual TrackingRecHit::ConstRecHitPointer makeShared(SiPixelRecHit const & hit, TrajectoryStateOnSurface const& tsos) const=0;

RecoTracker/TkSeedingLayers/src/HitExtractorSTRP.cc

@@ -82,7 +82,7 @@ HitExtractorSTRP::skipThis(const TkTransientTrackingRecHitBuilder& ttrhBuilder,
   // replace with one
 
   auto cloner = ttrhBuilder.cloner();
-  replaceMe = cloner.project(hit, rejectSt, TrajectoryStateOnSurface());
+  replaceMe = cloner.project(hit, rejectSt, TrajectoryStateOnSurface()).release();
   if (rejectSt)
     LogDebug("HitExtractorSTRP")<<"a matched hit is partially masked, and the mono hit got projected onto: "<<replaceMe->geographicalId().rawId()<<" key: "<<hit.monoClusterRef().key();
   else 

RecoTracker/TransientTrackingRecHit/interface/TkClonerImpl.h

@@ -1,9 +1,9 @@
 #ifndef TKClonerImplRecHit_H
 #define TKClonerImplRecHit_H
 
+#include <memory>
 #include "DataFormats/TrackerRecHit2D/interface/TkCloner.h"
 
-
 class PixelClusterParameterEstimator;
 class StripClusterParameterEstimator;
 class SiStripRecHitMatcher;
@@ -18,23 +18,23 @@ class TkClonerImpl final : public TkCloner {
 	       ): pixelCPE(ipixelCPE), stripCPE(istripCPE), theMatcher(iMatcher){}
 
   using TkCloner::operator();
-  virtual SiPixelRecHit * operator()(SiPixelRecHit const & hit, TrajectoryStateOnSurface const& tsos) const;
-  virtual SiStripRecHit2D * operator()(SiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const;
-  virtual SiStripRecHit1D * operator()(SiStripRecHit1D const & hit, TrajectoryStateOnSurface const& tsos) const;
-  virtual SiStripMatchedRecHit2D * operator()(SiStripMatchedRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const;
-  virtual ProjectedSiStripRecHit2D * operator()(ProjectedSiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const;
+  virtual std::unique_ptr<SiPixelRecHit> operator()(SiPixelRecHit const & hit, TrajectoryStateOnSurface const& tsos) const override;
+  virtual std::unique_ptr<SiStripRecHit2D> operator()(SiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const override;
+  virtual std::unique_ptr<SiStripRecHit1D> operator()(SiStripRecHit1D const & hit, TrajectoryStateOnSurface const& tsos) const override;
+  virtual std::unique_ptr<SiStripMatchedRecHit2D> operator()(SiStripMatchedRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const override;
+  virtual std::unique_ptr<ProjectedSiStripRecHit2D> operator()(ProjectedSiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const override;
 
 
   using TkCloner::makeShared;
-  virtual TrackingRecHit::ConstRecHitPointer makeShared(SiPixelRecHit const & hit, TrajectoryStateOnSurface const& tsos) const;
-  virtual TrackingRecHit::ConstRecHitPointer makeShared(SiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const;
-  virtual TrackingRecHit::ConstRecHitPointer makeShared(SiStripRecHit1D const & hit, TrajectoryStateOnSurface const& tsos) const;
-  virtual TrackingRecHit::ConstRecHitPointer makeShared(SiStripMatchedRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const;
-  virtual TrackingRecHit::ConstRecHitPointer makeShared(ProjectedSiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const;
+  virtual TrackingRecHit::ConstRecHitPointer makeShared(SiPixelRecHit const & hit, TrajectoryStateOnSurface const& tsos) const override;
+  virtual TrackingRecHit::ConstRecHitPointer makeShared(SiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const override;
+  virtual TrackingRecHit::ConstRecHitPointer makeShared(SiStripRecHit1D const & hit, TrajectoryStateOnSurface const& tsos) const override;
+  virtual TrackingRecHit::ConstRecHitPointer makeShared(SiStripMatchedRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const override;
+  virtual TrackingRecHit::ConstRecHitPointer makeShared(ProjectedSiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const override;
 
 
   // project either mono or stero hit...
-  ProjectedSiStripRecHit2D * project(SiStripMatchedRecHit2D const & hit, bool mono, TrajectoryStateOnSurface const& tsos) const;
+  std::unique_ptr<ProjectedSiStripRecHit2D> project(SiStripMatchedRecHit2D const & hit, bool mono, TrajectoryStateOnSurface const& tsos) const;
 
 private:
   const PixelClusterParameterEstimator * pixelCPE;

RecoTracker/TransientTrackingRecHit/src/TkClonerImpl.cc

@@ -21,27 +21,27 @@
 #include<iostream>
 #include <memory>
 
-SiPixelRecHit * TkClonerImpl::operator()(SiPixelRecHit const & hit, TrajectoryStateOnSurface const& tsos) const {
+std::unique_ptr<SiPixelRecHit>  TkClonerImpl::operator()(SiPixelRecHit const & hit, TrajectoryStateOnSurface const& tsos) const {
   const SiPixelCluster& clust = *hit.cluster();  
   auto && params = pixelCPE->getParameters( clust, *hit.detUnit(), tsos);
-  return new SiPixelRecHit(std::get<0>(params), std::get<1>(params), std::get<2>(params), *hit.det(), hit.cluster());
+  return std::unique_ptr<SiPixelRecHit>(new SiPixelRecHit(std::get<0>(params), std::get<1>(params), std::get<2>(params), *hit.det(), hit.cluster()));
 }
 
-SiStripRecHit2D * TkClonerImpl::operator()(SiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const {
+std::unique_ptr<SiStripRecHit2D> TkClonerImpl::operator()(SiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const {
     /// FIXME: this only uses the first cluster and ignores the others
     const SiStripCluster&  clust = hit.stripCluster();  
     StripClusterParameterEstimator::LocalValues lv = 
       stripCPE->localParameters( clust, *hit.detUnit(), tsos);
- return new SiStripRecHit2D(lv.first, lv.second, *hit.det(), hit.omniCluster());
+    return std::unique_ptr<SiStripRecHit2D>{new SiStripRecHit2D(lv.first, lv.second, *hit.det(), hit.omniCluster())};
 }
 
-SiStripRecHit1D * TkClonerImpl::operator()(SiStripRecHit1D const & hit, TrajectoryStateOnSurface const& tsos) const {
+std::unique_ptr<SiStripRecHit1D> TkClonerImpl::operator()(SiStripRecHit1D const & hit, TrajectoryStateOnSurface const& tsos) const {
   /// FIXME: this only uses the first cluster and ignores the others
   const SiStripCluster&  clust = hit.stripCluster();  
   StripClusterParameterEstimator::LocalValues lv = 
     stripCPE->localParameters( clust, *hit.detUnit(), tsos);
   LocalError le(lv.second.xx(),0.,std::numeric_limits<float>::max()); //Correct??
-  return new SiStripRecHit1D(lv.first, le, *hit.det(), hit.omniCluster());
+  return std::unique_ptr<SiStripRecHit1D>{new SiStripRecHit1D(lv.first, le, *hit.det(), hit.omniCluster())};
 }
 
 TrackingRecHit::ConstRecHitPointer TkClonerImpl::makeShared(SiPixelRecHit const & hit, TrajectoryStateOnSurface const& tsos) const {
@@ -100,7 +100,7 @@ namespace {
 #endif
 }
 
-SiStripMatchedRecHit2D * TkClonerImpl::operator()(SiStripMatchedRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const {
+std::unique_ptr<SiStripMatchedRecHit2D> TkClonerImpl::operator()(SiStripMatchedRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const {
     const GeomDet * det = hit.det();
     const GluedGeomDet *gdet = static_cast<const GluedGeomDet *> (det);
     LocalVector tkDir = (tsos.isValid() ? tsos.localDirection() : 
@@ -123,10 +123,10 @@ SiStripMatchedRecHit2D * TkClonerImpl::operator()(SiStripMatchedRecHit2D const &
 
     // return theMatcher->match(&monoHit,&stereoHit,gdet,tkDir,true);
     std::unique_ptr<SiStripMatchedRecHit2D> temp = theMatcher->match(&monoHit,&stereoHit,gdet,tkDir,false);
-    SiStripMatchedRecHit2D * better =  temp.release();
-
-    return better ? better : hit.clone();
-
+    if(temp.get() == nullptr) {
+      temp = std::unique_ptr<SiStripMatchedRecHit2D>(hit.clone());
+    }
+    return temp;
 }
 
 TrackingRecHit::ConstRecHitPointer TkClonerImpl::makeShared(SiStripMatchedRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const {
@@ -139,7 +139,7 @@ TrackingRecHit::ConstRecHitPointer TkClonerImpl::makeShared(ProjectedSiStripRecH
    return TrackingRecHit::ConstRecHitPointer((*this)(hit,tsos));
 }
 
-ProjectedSiStripRecHit2D * TkClonerImpl::operator()(ProjectedSiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const {
+std::unique_ptr<ProjectedSiStripRecHit2D> TkClonerImpl::operator()(ProjectedSiStripRecHit2D const & hit, TrajectoryStateOnSurface const& tsos) const {
   const SiStripCluster& clust = hit.stripCluster();
   const GeomDetUnit * gdu = reinterpret_cast<const GeomDetUnit *>(hit.originalDet());
   //if (!gdu) std::cout<<"no luck dude"<<std::endl;
@@ -164,11 +164,11 @@ ProjectedSiStripRecHit2D * TkClonerImpl::operator()(ProjectedSiStripRecHit2D con
     hitErr = LocalError( hitErr.xx(), -hitErr.xy(), hitErr.yy());
   }
   LocalError rotatedError = hitErr.rotate( hitXAxis.x(), hitXAxis.y());
-  return new ProjectedSiStripRecHit2D(projectedHitPos, rotatedError, *hit.det(), *hit.originalDet(), hit.omniCluster());
+  return std::unique_ptr<ProjectedSiStripRecHit2D>{new ProjectedSiStripRecHit2D(projectedHitPos, rotatedError, *hit.det(), *hit.originalDet(), hit.omniCluster())};
 }
 
 
-ProjectedSiStripRecHit2D * TkClonerImpl::project(SiStripMatchedRecHit2D const & hit, bool mono, TrajectoryStateOnSurface const& tsos) const {
+std::unique_ptr<ProjectedSiStripRecHit2D> TkClonerImpl::project(SiStripMatchedRecHit2D const & hit, bool mono, TrajectoryStateOnSurface const& tsos) const {
   const GeomDet & det = *hit.det();
   const GluedGeomDet & gdet = static_cast<const GluedGeomDet &> (det);
   const GeomDetUnit * odet = mono ? gdet.monoDet() : gdet.stereoDet();
@@ -203,6 +203,7 @@ ProjectedSiStripRecHit2D * TkClonerImpl::project(SiStripMatchedRecHit2D const &
     hitErr = LocalError( hitErr.xx(), -hitErr.xy(), hitErr.yy());
   }
   LocalError rotatedError = hitErr.rotate( hitXAxis.x(), hitXAxis.y());
-  return new ProjectedSiStripRecHit2D(projectedHitPos, rotatedError, det, *odet, 
-				      mono ? hit.monoClusterRef() : hit.stereoClusterRef() );
+  return std::unique_ptr<ProjectedSiStripRecHit2D>{
+    new ProjectedSiStripRecHit2D(projectedHitPos, rotatedError, det, *odet, 
+				 mono ? hit.monoClusterRef() : hit.stereoClusterRef() ) };
 }