[RF] Implement mechanism in RooCmdArg to take ownership of temporaries

It happens often that one passes a temporary RooArgSet to the
RooCmdArg constructor, where the RooCmdArg is itself a temporary that is
passed to a RooFit function. The RooCmdArg contains only pointers to the
RooArgSets, so this pattern relies on the RooArgSet being kept alive
until the function end.

In code, this pattern would look like this statement:
```C++
func(RooCmdArg{RooArgSet{}});
```

In C++, this works because the temporaries survive until the statement
end. But in Python, the inner temporary RooArgSet will be destructed
after the RooCmdArg has been constructed, leaving the RooCmdArg with
dangling pointers.

To solve this problem, all the functions that can create a RooCmdArg
from a RooArgSet are overloaded with versions accepting rvalue
references. These overloads are indeed used for temporaries both in C++
and Python. In the rvalue reference versions, the temporaries are moved
into a vector of std::unique_ptr<TObject>. The constructed RooCmdArg
then takes ownership of this vector wrapped in a shared_ptr.

Author: guitargeek

roofit/roofitcore/inc/RooCmdArg.h

@@ -98,6 +98,12 @@ class RooCmdArg : public TNamed {
 
   void Print(const char* = "") const;
 
+  template<class T>
+  static T const& take(T && obj) {
+    _nextSharedData.emplace_back(new T{std::move(obj)});
+    return static_cast<T const&>(*_nextSharedData.back());
+  }
+
 protected:
 
   static const RooCmdArg _none  ; // Static instance of null object
@@ -116,6 +122,12 @@ class RooCmdArg : public TNamed {
   RooArgSet* _c ;        // Payload RooArgSets 
   RooLinkedList _argList ; // Payload sub-arguments
   Bool_t _prefixSubArgs ; // Prefix subarguments with container name?
+
+  using DataCollection = std::vector<std::unique_ptr<TObject>>;
+  std::shared_ptr<DataCollection> _sharedData; //!
+
+  // the next RooCmdArg created will take ownership of this data
+  static DataCollection _nextSharedData;
 
   ClassDef(RooCmdArg,2) // Generic named argument container
 };

roofit/roofitcore/inc/RooGlobalFunc.h

@@ -19,10 +19,6 @@
 #include "RooCmdArg.h"
 #include <map>
 #include <string>
-//#include "RooDataHist.h"
-//#include "RooAbsPdf.h"
-//#include "RooRealConstant.h"
-//#include "RooMsgService.h"
 
 class RooDataHist ;
 class RooDataSet ;
@@ -46,9 +42,6 @@ class RooNumIntConfig ;
 class RooArgList ;
 class RooAbsCollection ;
 class TH1 ;
-class TF1 ;
-class TF2 ;
-class TF3 ;
 class TTree ;
 
 /*! \namespace RooFit
@@ -83,10 +76,13 @@ enum MPSplit { BulkPartition=0, Interleave=1, SimComponents=2, Hybrid=3 } ;
 RooCmdArg DrawOption(const char* opt) ;
 RooCmdArg Normalization(Double_t scaleFactor) ;
 RooCmdArg Slice(const RooArgSet& sliceSet) ;
+RooCmdArg Slice(RooArgSet && sliceSet) ;
 RooCmdArg Slice(RooCategory& cat, const char* label) ;
 RooCmdArg Project(const RooArgSet& projSet) ;
+RooCmdArg Project(RooArgSet && projSet) ;
 RooCmdArg ProjWData(const RooAbsData& projData, Bool_t binData=kFALSE) ;
 RooCmdArg ProjWData(const RooArgSet& projSet, const RooAbsData& projData, Bool_t binData=kFALSE) ;
+RooCmdArg ProjWData(RooArgSet && projSet, const RooAbsData& projData, Bool_t binData=kFALSE) ;
 RooCmdArg Asymmetry(const RooCategory& cat) ;
 RooCmdArg Precision(Double_t prec) ;
 RooCmdArg ShiftToZero() ;
@@ -108,11 +104,13 @@ RooCmdArg MoveToBack()  ;
 RooCmdArg VisualizeError(const RooDataSet& paramData, Double_t Z=1) ;
 RooCmdArg VisualizeError(const RooFitResult& fitres, Double_t Z=1, Bool_t linearMethod=kTRUE) ;
 RooCmdArg VisualizeError(const RooFitResult& fitres, const RooArgSet& param, Double_t Z=1, Bool_t linearMethod=kTRUE) ;
+RooCmdArg VisualizeError(const RooFitResult& fitres, RooArgSet && param, Double_t Z=1, Bool_t linearMethod=kTRUE) ;
 RooCmdArg ShowProgress() ;
 
 // RooAbsPdf::plotOn arguments
 RooCmdArg Normalization(Double_t scaleFactor, Int_t scaleType) ;
 RooCmdArg Components(const RooArgSet& compSet) ;
+RooCmdArg Components(RooArgSet && compSet) ;
 RooCmdArg Components(const char* compSpec) ;
 
 // RooAbsData::plotOn arguments
@@ -156,7 +154,9 @@ RooCmdArg Import(RooDataSet& data) ;
 RooCmdArg Import(TTree& tree) ;
 RooCmdArg ImportFromFile(const char* fname, const char* tname) ;
 RooCmdArg StoreError(const RooArgSet& aset) ;
+RooCmdArg StoreError(RooArgSet && aset) ;
 RooCmdArg StoreAsymError(const RooArgSet& aset) ;
+RooCmdArg StoreAsymError(RooArgSet && aset) ;
 RooCmdArg OwnLinked() ;
 
 /** @} */
@@ -183,9 +183,11 @@ RooCmdArg AutoBinning(Int_t nbins=100, Double_t marginFactor=0.1) ;
 
 // RooAbsReal::fillHistogram arguments
 RooCmdArg IntegratedObservables(const RooArgSet& intObs) ;
+RooCmdArg IntegratedObservables(RooArgSet && intObs) ;
 
 // RooAbsData::reduce arguments
 RooCmdArg SelectVars(const RooArgSet& vars) ;
+RooCmdArg SelectVars(RooArgSet && vars) ;
 RooCmdArg EventRange(Int_t nStart, Int_t nStop) ;
 
 
@@ -216,13 +218,17 @@ RooCmdArg InitialHesse(Bool_t flag=kTRUE) ;
 RooCmdArg Hesse(Bool_t flag=kTRUE) ;
 RooCmdArg Minos(Bool_t flag=kTRUE) ;
 RooCmdArg Minos(const RooArgSet& minosArgs) ;
+RooCmdArg Minos(RooArgSet && minosArgs) ;
 RooCmdArg SplitRange(Bool_t flag=kTRUE) ;
 RooCmdArg SumCoefRange(const char* rangeName) ;
 RooCmdArg Constrain(const RooArgSet& params) ;
+RooCmdArg Constrain(RooArgSet && params) ;
 RooCmdArg GlobalObservables(const RooArgSet& globs) ;
+RooCmdArg GlobalObservables(RooArgSet && globs) ;
 RooCmdArg GlobalObservablesTag(const char* tagName) ;
 //RooCmdArg Constrained() ;
 RooCmdArg ExternalConstraints(const RooArgSet& constraintPdfs) ;
+RooCmdArg ExternalConstraints(RooArgSet && constraintPdfs) ;
 RooCmdArg PrintEvalErrors(Int_t numErrors) ;
 RooCmdArg EvalErrorWall(Bool_t flag) ;
 RooCmdArg SumW2Error(Bool_t flag) ;
@@ -238,13 +244,17 @@ RooCmdArg RecoverFromUndefinedRegions(double strength);
 RooCmdArg Label(const char* str) ;
 RooCmdArg Layout(Double_t xmin, Double_t xmax=0.99, Double_t ymin=0.95) ;
 RooCmdArg Parameters(const RooArgSet& params) ;
+RooCmdArg Parameters(RooArgSet && params) ;
 RooCmdArg ShowConstants(Bool_t flag=kTRUE) ;
 
 // RooTreeData::statOn arguments
 RooCmdArg What(const char* str) ;
 
 // RooProdPdf::ctor arguments
 RooCmdArg Conditional(const RooArgSet& pdfSet, const RooArgSet& depSet, Bool_t depsAreCond=kFALSE) ;
+RooCmdArg Conditional(RooArgSet && pdfSet, const RooArgSet& depSet, Bool_t depsAreCond=kFALSE) ;
+RooCmdArg Conditional(const RooArgSet& pdfSet, RooArgSet && depSet, Bool_t depsAreCond=kFALSE) ;
+RooCmdArg Conditional(RooArgSet && pdfSet, RooArgSet && depSet, Bool_t depsAreCond=kFALSE) ;
 
 /**
  * \defgroup Generating Arguments for generating data
@@ -274,6 +284,7 @@ RooCmdArg IntrinsicBinning(Bool_t flag=kTRUE) ;
 
 // RooAbsReal::createIntegral arguments
 RooCmdArg NormSet(const RooArgSet& nset) ;
+RooCmdArg NormSet(RooArgSet && nset) ;
 RooCmdArg NumIntConfig(const RooNumIntConfig& cfg) ;
 
 // RooMCStudy::ctor arguments
@@ -330,6 +341,7 @@ RooCmdArg Restrict(const char* catName, const char* stateNameList) ;
 
 // RooAbsPdf::createCdf() arguments
 RooCmdArg SupNormSet(const RooArgSet& nset) ;
+RooCmdArg SupNormSet(RooArgSet && nset) ;
 RooCmdArg ScanParameters(Int_t nbins,Int_t intOrder) ;
 RooCmdArg ScanNumCdf() ;
 RooCmdArg ScanAllCdf() ;

roofit/roofitcore/src/RooCmdArg.cxx

@@ -101,6 +101,9 @@ RooCmdArg::RooCmdArg(const char* name, Int_t i1, Int_t i2, Double_t d1, Double_t
   if (ca) {
     _argList.Add(new RooCmdArg(*ca)) ;
   }
+
+  _sharedData.reset(new DataCollection);
+  _sharedData->swap(_nextSharedData);
 }
 
 
@@ -109,7 +112,8 @@ RooCmdArg::RooCmdArg(const char* name, Int_t i1, Int_t i2, Double_t d1, Double_t
 /// Copy constructor
 
 RooCmdArg::RooCmdArg(const RooCmdArg& other) :
-  TNamed(other)
+  TNamed(other),
+  _sharedData{other._sharedData}
 {
   _i[0] = other._i[0] ;
   _i[1] = other._i[1] ;
@@ -143,6 +147,8 @@ RooCmdArg& RooCmdArg::operator=(const RooCmdArg& other)
 {
   if (&other==this) return *this ;
 
+  _sharedData = other._sharedData;
+
   SetName(other.GetName()) ;
   SetTitle(other.GetTitle()) ;
 
@@ -224,3 +230,5 @@ void RooCmdArg::Print(const char*) const {
       << "\nstrings\t" << _s[0] << " " << _s[1] << " " << _s[2]
       << "\nobjects\t" << _o[0] << " " << _o[1] << std::endl;
 }
+
+RooCmdArg::DataCollection RooCmdArg::_nextSharedData = RooCmdArg::DataCollection{};

roofit/roofitcore/src/RooGlobalFunc.cxx

@@ -38,12 +38,17 @@ namespace RooFit {
   // RooAbsReal::plotOn arguments
   RooCmdArg DrawOption(const char* opt)            { return RooCmdArg("DrawOption",0,0,0,0,opt,0,0,0) ; }
   RooCmdArg Slice(const RooArgSet& sliceSet)       { return RooCmdArg("SliceVars",0,0,0,0,0,0,&sliceSet,0) ; }
+  RooCmdArg Slice(RooArgSet && sliceSet)           { return Slice(RooCmdArg::take(std::move(sliceSet))); }
   RooCmdArg Slice(RooCategory& cat, const char* label) { return RooCmdArg("SliceCat",0,0,0,0,label,0,&cat,0) ; }
 
   RooCmdArg Project(const RooArgSet& projSet)      { return RooCmdArg("Project",0,0,0,0,0,0,&projSet,0) ; }
+  RooCmdArg Project(RooArgSet && projSet)          { return Project(RooCmdArg::take(std::move(projSet))); }
   RooCmdArg ProjWData(const RooArgSet& projSet, 
                       const RooAbsData& projData,
                       Bool_t binData)              { return RooCmdArg("ProjData",binData,0,0,0,0,0,&projSet,&projData) ; }
+  RooCmdArg ProjWData(RooArgSet && projSet, const RooAbsData& projData, Bool_t binData) {
+    return ProjWData(RooCmdArg::take(std::move(projSet)), projData, binData);
+  }
   RooCmdArg ProjWData(const RooAbsData& projData,
                       Bool_t binData)              { return RooCmdArg("ProjData",binData,0,0,0,0,0,0,&projData) ; }
   RooCmdArg Asymmetry(const RooCategory& cat)      { return RooCmdArg("Asymmetry",0,0,0,0,0,0,&cat,0) ; }
@@ -68,11 +73,15 @@ namespace RooFit {
   RooCmdArg VisualizeError(const RooFitResult& fitres, Double_t Z, Bool_t EVmethod)  { return RooCmdArg("VisualizeError",EVmethod,0,Z,0,0,0,&fitres,0) ; }
   RooCmdArg VisualizeError(const RooFitResult& fitres, const RooArgSet& param, Double_t Z, Bool_t EVmethod) 
                                                                   { return RooCmdArg("VisualizeError",EVmethod,0,Z,0,0,0,&fitres,0,0,0,&param) ; }
+  RooCmdArg VisualizeError(const RooFitResult& fitres, RooArgSet && param, Double_t Z, Bool_t linearMethod) {
+    return VisualizeError(fitres, RooCmdArg::take(std::move(param)), Z, linearMethod);
+  }
   RooCmdArg VisualizeError(const RooDataSet& paramData, Double_t Z) { return RooCmdArg("VisualizeErrorData",0,0,Z,0,0,0,&paramData,0) ; }
   RooCmdArg ShowProgress()                         { return RooCmdArg("ShowProgress",1,0,0,0,0,0,0,0) ; }
 
   // RooAbsPdf::plotOn arguments
   RooCmdArg Components(const RooArgSet& compSet) { return RooCmdArg("SelectCompSet",0,0,0,0,0,0,&compSet,0) ; }
+  RooCmdArg Components(RooArgSet && compSet) { return Components(RooCmdArg::take(std::move(compSet))); }
   RooCmdArg Components(const char* compSpec) { return RooCmdArg("SelectCompSpec",0,0,0,0,compSpec,0,0,0) ; }
   RooCmdArg Normalization(Double_t scaleFactor, Int_t scaleType) 
                                                    { return RooCmdArg("Normalization",scaleType,0,scaleFactor,0,0,0,0,0) ; }
@@ -129,7 +138,9 @@ namespace RooFit {
   RooCmdArg Import(TTree& tree)                         { return RooCmdArg("ImportTree",0,0,0,0,0,0,reinterpret_cast<TObject*>(&tree),0) ; }
   RooCmdArg ImportFromFile(const char* fname, const char* tname){ return RooCmdArg("ImportFromFile",0,0,0,0,fname,tname,0,0) ; }
   RooCmdArg StoreError(const RooArgSet& aset)           { return RooCmdArg("StoreError",0,0,0,0,0,0,0,0,0,0,&aset) ; }
+  RooCmdArg StoreError(RooArgSet && aset)               { return StoreError(RooCmdArg::take(std::move(aset))); }
   RooCmdArg StoreAsymError(const RooArgSet& aset)       { return RooCmdArg("StoreAsymError",0,0,0,0,0,0,0,0,0,0,&aset) ; }
+  RooCmdArg StoreAsymError(RooArgSet && aset)           { return StoreAsymError(RooCmdArg::take(std::move(aset))); }
   RooCmdArg OwnLinked()                                 { return RooCmdArg("OwnLinked",1,0,0,0,0,0,0,0,0,0,0) ; }
 
   RooCmdArg Import(const std::map<std::string,RooDataSet*>& arg) {
@@ -180,6 +191,7 @@ namespace RooFit {
 
   // RooAbsData::reduce arguments
   RooCmdArg SelectVars(const RooArgSet& vars)     { return RooCmdArg("SelectVars",0,0,0,0,0,0,&vars,0) ; }
+  RooCmdArg SelectVars(RooArgSet && vars) { return SelectVars(RooCmdArg::take(std::move(vars))); }
   RooCmdArg EventRange(Int_t nStart, Int_t nStop) { return RooCmdArg("EventRange",nStart,nStop,0,0,0,0,0,0) ; }
 
   // RooAbsPdf::fitTo arguments
@@ -196,15 +208,19 @@ namespace RooFit {
   RooCmdArg Hesse(Bool_t flag)           { return RooCmdArg("Hesse",flag,0,0,0,0,0,0,0) ; }
   RooCmdArg Minos(Bool_t flag)           { return RooCmdArg("Minos",flag,0,0,0,0,0,0,0) ; }
   RooCmdArg Minos(const RooArgSet& minosArgs)            { return RooCmdArg("Minos",kTRUE,0,0,0,0,0,&minosArgs,0) ; }
+  RooCmdArg Minos(RooArgSet && minosArgs) { return Minos(RooCmdArg::take(std::move(minosArgs))); }
   RooCmdArg ConditionalObservables(const RooArgSet& set) { return RooCmdArg("ProjectedObservables",0,0,0,0,0,0,0,0,0,0,&set) ; }
   RooCmdArg ProjectedObservables(const RooArgSet& set)   { return RooCmdArg("ProjectedObservables",0,0,0,0,0,0,0,0,0,0,&set) ; }
   RooCmdArg SplitRange(Bool_t flag)                      { return RooCmdArg("SplitRange",flag,0,0,0,0,0,0,0) ; }
   RooCmdArg SumCoefRange(const char* rangeName)          { return RooCmdArg("SumCoefRange",0,0,0,0,rangeName,0,0,0) ; }
   RooCmdArg Constrain(const RooArgSet& params)           { return RooCmdArg("Constrain",0,0,0,0,0,0,0,0,0,0,&params) ; }
+  RooCmdArg Constrain(RooArgSet && params) { return Constrain(RooCmdArg::take(std::move(params))); }
   RooCmdArg GlobalObservables(const RooArgSet& globs)    { return RooCmdArg("GlobalObservables",0,0,0,0,0,0,0,0,0,0,&globs) ; }
+  RooCmdArg GlobalObservables(RooArgSet && globs) { return GlobalObservables(RooCmdArg::take(std::move(globs))); }
   RooCmdArg GlobalObservablesTag(const char* tagName)    { return RooCmdArg("GlobalObservablesTag",0,0,0,0,tagName,0,0,0) ; }
 //  RooCmdArg Constrained()                                { return RooCmdArg("Constrained",kTRUE,0,0,0,0,0,0,0) ; }
   RooCmdArg ExternalConstraints(const RooArgSet& cpdfs)  { return RooCmdArg("ExternalConstraints",0,0,0,0,0,0,&cpdfs,0,0,0,&cpdfs) ; }
+  RooCmdArg ExternalConstraints(RooArgSet && cpdfs)      { return ExternalConstraints(RooCmdArg::take(std::move(cpdfs))); }
   RooCmdArg PrintEvalErrors(Int_t numErrors)             { return RooCmdArg("PrintEvalErrors",numErrors,0,0,0,0,0,0,0) ; }
   RooCmdArg EvalErrorWall(Bool_t flag)                   { return RooCmdArg("EvalErrorWall",flag,0,0,0,0,0,0,0) ; }
   RooCmdArg SumW2Error(Bool_t flag)                      { return RooCmdArg("SumW2Error",flag,0,0,0,0,0,0,0) ; }
@@ -222,13 +238,23 @@ namespace RooFit {
   RooCmdArg Label(const char* str) { return RooCmdArg("Label",0,0,0,0,str,0,0,0) ; }
   RooCmdArg Layout(Double_t xmin, Double_t xmax, Double_t ymin) { return RooCmdArg("Layout",Int_t(ymin*10000),0,xmin,xmax,0,0,0,0) ; }
   RooCmdArg Parameters(const RooArgSet& params) { return RooCmdArg("Parameters",0,0,0,0,0,0,&params,0) ; }
+  RooCmdArg Parameters(RooArgSet && params) { return Parameters(RooCmdArg::take(std::move(params))) ; }
   RooCmdArg ShowConstants(Bool_t flag) { return RooCmdArg("ShowConstants",flag,0,0,0,0,0,0,0) ; }
 
   // RooTreeData::statOn arguments
   RooCmdArg What(const char* str) { return RooCmdArg("What",0,0,0,0,str,0,0,0) ; }
 
   // RooProdPdf::ctor arguments
   RooCmdArg Conditional(const RooArgSet& pdfSet, const RooArgSet& depSet, Bool_t depsAreCond) { return RooCmdArg("Conditional",depsAreCond,0,0,0,0,0,0,0,0,0,&pdfSet,&depSet) ; } ;
+  RooCmdArg Conditional(RooArgSet && pdfSet, const RooArgSet& depSet, Bool_t depsAreCond) {
+    return Conditional(RooCmdArg::take(std::move(pdfSet)), depSet, depsAreCond);
+  }
+  RooCmdArg Conditional(const RooArgSet& pdfSet, RooArgSet && depSet, Bool_t depsAreCond) {
+    return Conditional(pdfSet, RooCmdArg::take(std::move(depSet)), depsAreCond);
+  }
+  RooCmdArg Conditional(RooArgSet && pdfSet, RooArgSet && depSet, Bool_t depsAreCond) {
+    return Conditional(RooCmdArg::take(std::move(pdfSet)), RooCmdArg::take(std::move(depSet)), depsAreCond);
+  }
 
   // RooAbsPdf::generate arguments
   RooCmdArg ProtoData(const RooDataSet& protoData, Bool_t randomizeOrder, Bool_t resample) 
@@ -257,9 +283,11 @@ namespace RooFit {
 
   // RooAbsReal::fillHistogram arguments
   RooCmdArg IntegratedObservables(const RooArgSet& intObs) {  return RooCmdArg("IntObs",0,0,0,0,0,0,0,0,0,0,&intObs,0) ; } ;  
+  RooCmdArg IntegratedObservables(RooArgSet && intObs) { return IntegratedObservables(RooCmdArg::take(std::move(intObs))); }
 
   // RooAbsReal::createIntegral arguments
   RooCmdArg NormSet(const RooArgSet& nset)           { return RooCmdArg("NormSet",0,0,0,0,0,0,&nset,0) ; }
+  RooCmdArg NormSet(RooArgSet && nset) { return NormSet(RooCmdArg::take(std::move(nset))); }
   RooCmdArg NumIntConfig(const RooNumIntConfig& cfg) { return RooCmdArg("NumIntConfig",0,0,0,0,0,0,&cfg,0) ; }
 
   // RooMCStudy::ctor arguments
@@ -331,6 +359,7 @@ namespace RooFit {
 
   // RooAbsPdf::createCdf() arguments
   RooCmdArg SupNormSet(const RooArgSet& nset) { return RooCmdArg("SupNormSet",0,0,0,0,0,0,&nset,0) ; } 
+  RooCmdArg SupNormSet(RooArgSet && nset) { return SupNormSet(RooCmdArg::take(std::move(nset))); }
   RooCmdArg ScanParameters(Int_t nbins,Int_t intOrder) { return RooCmdArg("ScanParameters",nbins,intOrder,0,0,0,0,0,0) ; }
   RooCmdArg ScanNumCdf() { return RooCmdArg("ScanNumCdf",1,0,0,0,0,0,0,0) ; }
   RooCmdArg ScanAllCdf() { return RooCmdArg("ScanAllCdf",1,0,0,0,0,0,0,0) ; }