[RF] Avoid using RooAbsArg::redirectServer() in RooFit

roofit/histfactory/src/HistFactoryNavigation.cxx

@@ -1012,6 +1012,12 @@ namespace RooStats {
         throw hf_exc();
       }
 
+
+      const std::string attrib = "ORIGNAME:" + ToReplace;
+      const bool oldAttrib = ReplaceWith->getAttribute(attrib.c_str());
+      ReplaceWith->setAttribute(attrib.c_str());
+      RooArgSet newServerSet{*ReplaceWith};
+
       // Now that we have the node we want to replace, we have to
       // get its parent node
 
@@ -1025,14 +1031,15 @@ namespace RooStats {
         if( findChild(client->GetName(), fModel) == nullptr) continue;
 
         // Now, do the replacement:
-        bool valueProp=false;
-        bool shapeProp=false;
-        client->replaceServer( *nodeToReplace, *ReplaceWith, valueProp, shapeProp );
+        client->redirectServers(newServerSet);
         std::cout << "Replaced: " << ToReplace << " with: " << ReplaceWith->GetName()
                 << " in node: " << client->GetName() << std::endl;
 
       }
 
+      // reset temporary attribute for server redirection
+      ReplaceWith->setAttribute(attrib.c_str(), oldAttrib);
+
       return;
 
     }

roofit/jsoninterface/src/JSONParser.h

@@ -80,7 +80,7 @@ class TJSONTree : public RooFit::Detail::JSONTree {
 
 public:
    TJSONTree();
-   virtual ~TJSONTree();
+   ~TJSONTree() override;
    TJSONTree(std::istream &is);
    TJSONTree::Node &incache(const TJSONTree::Node &n);
 

roofit/jsoninterface/src/RYMLParser.h

@@ -35,33 +35,33 @@ class TRYMLTree : public RooFit::Detail::JSONTree {
       std::unique_ptr<Impl> node;
 
    public:
-      virtual void writeJSON(std::ostream &os) const override;
-      virtual void writeYML(std::ostream &) const override;
+      void writeJSON(std::ostream &os) const override;
+      void writeYML(std::ostream &) const override;
 
       Node(TRYMLTree *t, const Impl &other);
       Node(const Node &other);
-      virtual Node &operator<<(std::string const &s) override;
-      virtual Node &operator<<(int i) override;
-      virtual Node &operator<<(double d) override;
-      virtual const Node &operator>>(std::string &v) const override;
-      virtual Node &operator[](std::string const &k) override;
-      virtual Node &operator[](size_t pos) override;
-      virtual const Node &operator[](std::string const &k) const override;
-      virtual const Node &operator[](size_t pos) const override;
-      virtual bool is_container() const override;
-      virtual bool is_map() const override;
-      virtual bool is_seq() const override;
-      virtual void set_map() override;
-      virtual void set_seq() override;
-      virtual std::string key() const override;
-      virtual std::string val() const override;
-      virtual bool has_key() const override;
-      virtual bool has_val() const override;
-      virtual bool has_child(std::string const &) const override;
-      virtual Node &append_child() override;
-      virtual size_t num_children() const override;
-      virtual Node &child(size_t pos) override;
-      virtual const Node &child(size_t pos) const override;
+      Node &operator<<(std::string const &s) override;
+      Node &operator<<(int i) override;
+      Node &operator<<(double d) override;
+      const Node &operator>>(std::string &v) const override;
+      Node &operator[](std::string const &k) override;
+      Node &operator[](size_t pos) override;
+      const Node &operator[](std::string const &k) const override;
+      const Node &operator[](size_t pos) const override;
+      bool is_container() const override;
+      bool is_map() const override;
+      bool is_seq() const override;
+      void set_map() override;
+      void set_seq() override;
+      std::string key() const override;
+      std::string val() const override;
+      bool has_key() const override;
+      bool has_val() const override;
+      bool has_child(std::string const &) const override;
+      Node &append_child() override;
+      size_t num_children() const override;
+      Node &child(size_t pos) override;
+      const Node &child(size_t pos) const override;
    };
 
 protected:
@@ -75,9 +75,9 @@ class TRYMLTree : public RooFit::Detail::JSONTree {
 
 public:
    TRYMLTree();
-   ~TRYMLTree();
+   ~TRYMLTree() override;
    TRYMLTree(std::istream &is);
-   Node &rootnode();
+   Node &rootnode() override;
 };
 
 #endif

roofit/roofit/inc/RooExpPoly.h

@@ -21,8 +21,8 @@ class RooExpPoly : public RooAbsPdf {
    RooExpPoly(const char *name, const char *title, RooAbsReal &x);
    RooExpPoly(const char *name, const char *title, RooAbsReal &x, const RooArgList &coefList, int lowestOrder = 1);
 
-   RooExpPoly(const RooExpPoly &other, const char *name = 0);
-   virtual TObject *clone(const char *newname) const override { return new RooExpPoly(*this, newname); }
+   RooExpPoly(const RooExpPoly &other, const char *name = nullptr);
+   TObject *clone(const char *newname) const override { return new RooExpPoly(*this, newname); }
 
    double getLogVal(const RooArgSet *nset) const override;
 

roofit/roofit/inc/RooPower.h

@@ -24,7 +24,7 @@ class RooPower : public RooAbsPdf {
    RooPower(const char *name, const char *title, RooAbsReal &x, const RooArgList &coefList, const RooArgList &expList);
 
    RooPower(const RooPower &other, const char *name = nullptr);
-   virtual TObject *clone(const char *newname) const override { return new RooPower(*this, newname); }
+   TObject *clone(const char *newname) const override { return new RooPower(*this, newname); }
 
    std::string getFormulaExpression(bool expand) const;
 

roofit/roofitcore/inc/RooFit/TestStatistics/RooBinnedL.h

@@ -31,18 +31,18 @@ namespace TestStatistics {
 class RooBinnedL : public RooAbsL {
 public:
    RooBinnedL(RooAbsPdf *pdf, RooAbsData *data);
-   ~RooBinnedL();
+   ~RooBinnedL() override;
    ROOT::Math::KahanSum<double>
    evaluatePartition(Section bins, std::size_t components_begin, std::size_t components_end) override;
 
-   virtual std::string GetClassName() const override { return "RooBinnedL"; };
+   std::string GetClassName() const override { return "RooBinnedL"; }
 
 private:
    mutable bool _first = true;        ///<!
    mutable std::vector<double> _binw; ///<!
    std::unique_ptr<RooChangeTracker> paramTracker_;
-   Section lastSection_ = {0, 0};  // used for cache together with the parameter tracker
-   mutable ROOT::Math::KahanSum<double> cachedResult_ {0.};
+   Section lastSection_ = {0, 0}; // used for cache together with the parameter tracker
+   mutable ROOT::Math::KahanSum<double> cachedResult_{0.};
 };
 
 } // namespace TestStatistics

roofit/roofitcore/inc/RooFit/TestStatistics/RooSubsidiaryL.h

@@ -36,9 +36,9 @@ class RooSubsidiaryL : public RooAbsL {
       return std::string("Subsidiary PDF set of simultaneous PDF ") + parent_pdf_name_;
    }
 
-   virtual std::string GetInfo() const override { return GetClassName() + "::" + parent_pdf_name_; }
+   std::string GetInfo() const override { return GetClassName() + "::" + parent_pdf_name_; }
 
-   virtual std::string GetClassName() const override { return "RooSubsidiaryL"; };
+   std::string GetClassName() const override { return "RooSubsidiaryL"; }
 
    inline std::size_t numDataEntries() const override
    {

roofit/roofitcore/inc/RooFit/TestStatistics/RooSumL.h

@@ -38,9 +38,9 @@ class RooSumL : public RooAbsL {
 
    void constOptimizeTestStatistic(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) override;
 
-   virtual std::string GetClassName() const  override { return "RooSumL"; };
+   std::string GetClassName() const override { return "RooSumL"; }
 
-   const std::vector<std::unique_ptr<RooAbsL>>& GetComponents() const  { return components_; };
+   const std::vector<std::unique_ptr<RooAbsL>> &GetComponents() const { return components_; };
 
 private:
    std::vector<std::unique_ptr<RooAbsL>> components_;

roofit/roofitcore/inc/RooFit/TestStatistics/RooUnbinnedL.h

@@ -34,23 +34,23 @@ class RooUnbinnedL : public RooAbsL {
    RooUnbinnedL(RooAbsPdf *pdf, RooAbsData *data, RooAbsL::Extended extended = RooAbsL::Extended::Auto,
                 bool useBatchedEvaluations = false);
    RooUnbinnedL(const RooUnbinnedL &other);
-   ~RooUnbinnedL();
+   ~RooUnbinnedL() override;
    bool setApplyWeightSquared(bool flag);
 
    ROOT::Math::KahanSum<double>
    evaluatePartition(Section events, std::size_t components_begin, std::size_t components_end) override;
 
    void setUseBatchedEvaluations(bool flag);
 
-   virtual std::string GetClassName() const override { return "RooUnbinnedL"; };
+   std::string GetClassName() const override { return "RooUnbinnedL"; }
 
 private:
-   bool apply_weight_squared = false;                              ///< Apply weights squared?
-   mutable bool _first = true;                                     ///<!
+   bool apply_weight_squared = false; ///< Apply weights squared?
+   mutable bool _first = true;        ///<!
    bool useBatchedEvaluations_ = false;
    std::unique_ptr<RooChangeTracker> paramTracker_;
-   Section lastSection_ = {0, 0};  // used for cache together with the parameter tracker
-   mutable ROOT::Math::KahanSum<double> cachedResult_ {0.};
+   Section lastSection_ = {0, 0}; // used for cache together with the parameter tracker
+   mutable ROOT::Math::KahanSum<double> cachedResult_{0.};
 };
 
 } // namespace TestStatistics

roofit/roofitcore/src/RooAbsAnaConvPdf.cxx

@@ -213,29 +213,31 @@ bool RooAbsAnaConvPdf::changeModel(const RooResolutionModel& newModel)
     auto conv = static_cast<RooResolutionModel*>(convArg);
 
     // Build new resolution model
-    RooResolutionModel* newConv = newModel.convolution((RooFormulaVar*)&conv->basis(),this) ;
-    if (!newConvSet.add(*newConv)) {
+    std::unique_ptr<RooResolutionModel> newConv{newModel.convolution(const_cast<RooFormulaVar*>(&conv->basis()),this)};
+    if (!newConvSet.addOwned(std::move(newConv))) {
       allOK = false ;
       break ;
     }
   }
 
   // Check if all convolutions were successfully built
   if (!allOK) {
-    // Delete new basis functions created sofar
-    std::for_each(newConvSet.begin(), newConvSet.end(), [](RooAbsArg* arg){delete arg;});
-
     return true ;
   }
 
   // Replace old convolutions with new set
   _convSet.removeAll() ;
-  _convSet.addOwned(newConvSet) ;
+  _convSet.addOwned(std::move(newConvSet));
+
+  const std::string attrib = std::string("ORIGNAME:") + _model->GetName();
+  const bool oldAttrib = newModel.getAttribute(attrib.c_str());
+  const_cast<RooResolutionModel&>(newModel).setAttribute(attrib.c_str());
 
-  // Update server link by hand, since _model.setArg() below will not do this
-  replaceServer((RooAbsArg&)_model.arg(),(RooAbsArg&)newModel,false,false) ;
+  redirectServers(RooArgSet{newModel}, false, true);
+
+  // reset temporary attribute for server redirection
+  const_cast<RooResolutionModel&>(newModel).setAttribute(attrib.c_str(), oldAttrib);
 
-  _model.setArg((RooResolutionModel&)newModel) ;
   return false ;
 }
 
@@ -671,4 +673,4 @@ void RooAbsAnaConvPdf::setCacheAndTrackHints(RooArgSet& trackNodes)
       //cout << "tracking node RooAddPdf component " << carg->ClassName() << "::" << carg->GetName() << endl ;
     }
   }
-}
+}