[RF] Implement RooLinkedList::begin(), end(), rbegin(), rend()

Implements `RooLinkedList::begin()`, `end()`, `rbegin()`, `rend()` and
extends the `RooLinkedListIterImpl` interface to support range-based
loops for the `RooLinkedList`.

Range-based loops on `RooLinkedList` are also used in some places in
RooFit to test this new feature. In particular the function
`RooCmdConfig::process` is used in basically every RooFit script.

This commit also adds `RooLinkedList::size()` and `empty()` for better
compatibility with STL containers and automatic pythonizations.

bindings/pyroot/pythonizations/test/CMakeLists.txt

@@ -128,6 +128,8 @@ if(roofit)
   ROOT_ADD_PYUNITTEST(pyroot_roofit_rooworkspace roofit/rooworkspace.py)
   ROOT_ADD_PYUNITTEST(pyroot_roofit_rooabspdf_fitto roofit/rooabspdf_fitto.py)
   ROOT_ADD_PYUNITTEST(pyroot_roofit_rooabsreal_ploton roofit/rooabsreal_ploton.py)
+
+  ROOT_ADD_PYUNITTEST(pyroot_roofit_roolinkedlist roofit/roolinkedlist.py)
 endif()
 
 if (dataframe)

bindings/pyroot/pythonizations/test/roofit/roolinkedlist.py

@@ -0,0 +1,33 @@
+import unittest
+
+import ROOT
+
+
+class TestRooLinkedList(unittest.TestCase):
+    """
+    Tests for the RooLinkedList.
+    """
+
+    def test_roolinkedlist_iteration(self):
+        # test if we can correctly iterate over a RooLinkedList, also in
+        # reverse.
+
+        roolist = ROOT.RooLinkedList()
+        pylist = []
+
+        n_elements = 3
+
+        for i in range(n_elements):
+            obj = ROOT.TNamed(str(i), str(i))
+            ROOT.SetOwnership(obj, False)
+            roolist.Add(obj)
+            pylist.append(obj)
+
+        self.assertEqual(len(roolist), n_elements)
+
+        for i, obj in enumerate(roolist):
+            self.assertEqual(str(i), obj.GetName())
+
+
+if __name__ == "__main__":
+    unittest.main()

roofit/roofitcore/inc/RooLinkedList.h

@@ -25,6 +25,7 @@
 #include <unordered_map>
 
 class RooLinkedListIter ;
+class RooLinkedListIterImpl ;
 class RooFIter;
 class TIterator ;
 class RooAbsArg ;
@@ -60,6 +61,8 @@ class RooLinkedList : public TObject {
   virtual ~RooLinkedList() ;
 
   Int_t GetSize() const { return _size ; }
+  std::size_t size() const { return _size ; }
+  bool empty() const { return _size == 0 ; }
 
   virtual void Add(TObject* arg) { Add(arg,1) ; }
   virtual Bool_t Remove(TObject* arg) ;
@@ -68,6 +71,10 @@ class RooLinkedList : public TObject {
   TIterator* MakeIterator(Bool_t forward = kTRUE) const ;
   RooLinkedListIter iterator(Bool_t forward = kTRUE) const ;
   RooFIter fwdIterator() const ;
+  RooLinkedListIterImpl begin() const;
+  RooLinkedListIterImpl end() const;
+  RooLinkedListIterImpl rbegin() const;
+  RooLinkedListIterImpl rend() const;
 
   void Clear(Option_t *o=0) ;
   void Delete(Option_t *o=0) ;

roofit/roofitcore/inc/RooLinkedListIter.h

@@ -143,7 +143,7 @@ class TIteratorToSTLInterface final : public TIterator , public GenericRooFIter
 
   }
 
-  Bool_t operator!=(const TIterator & other) const override {
+  bool operator!=(const TIterator & other) const override {
     const auto * castedOther =
         dynamic_cast<const TIteratorToSTLInterface<STLContainer>*>(&other);
     return !castedOther || &fSTLContainer != &(castedOther->fSTLContainer)
@@ -228,7 +228,7 @@ class RooLinkedListIter final : public TIterator {
 
   TObject * Next() override {return fIterImpl->Next();}
   void Reset() override {fIterImpl->Reset();}
-  Bool_t operator!=(const TIterator & other) const override {return fIterImpl->operator!=(other);}
+  bool operator!=(const TIterator & other) const override {return fIterImpl->operator!=(other);}
   TObject * operator*() const override {return fIterImpl->operator*();}
 
   private:
@@ -242,24 +242,11 @@ class RooLinkedListIter final : public TIterator {
 class RooLinkedListIterImpl final : public TIterator {
 public:
 
-  RooLinkedListIterImpl() {
-    // coverity[UNINIT_CTOR]
-  } ;
+  RooLinkedListIterImpl(const RooLinkedList* list, const RooLinkedListElem* ptr, bool forward) :
+    _list(list), _ptr(ptr), _forward(forward) {}
 
-
-  RooLinkedListIterImpl(const RooLinkedList* list, Bool_t forward) :
-    TIterator(), _list(list), _ptr(forward ? _list->_first : _list->_last),
-      _forward(forward)
-  { }
-
-  RooLinkedListIterImpl(const RooLinkedListIterImpl& other) :
-    TIterator(other), _list(other._list), _ptr(other._ptr),
-    _forward(other._forward)
-  {
-    // Copy constructor
-  }
-
-  virtual ~RooLinkedListIterImpl() { }
+  RooLinkedListIterImpl(const RooLinkedList* list, bool forward) :
+    RooLinkedListIterImpl(list, forward ? list->_first : list->_last, forward) {}
 
   TIterator& operator=(const TIterator& other) {
 
@@ -282,21 +269,17 @@ class RooLinkedListIterImpl final : public TIterator {
 
   virtual TObject *Next() {
     // Return next element in collection
-    if (!_ptr) return 0 ;
-    TObject* arg = _ptr->_arg ;
-    _ptr = _forward ? _ptr->_next : _ptr->_prev ;
-    return arg ;
+    return NextNV();
   }
 
   TObject *NextNV() {
     // Return next element in collection
-    if (!_ptr) return 0 ;
+    if (!_ptr) return nullptr ;
     TObject* arg = _ptr->_arg ;
     _ptr = _forward ? _ptr->_next : _ptr->_prev ;
     return arg ;
   }
 
-
   virtual void Reset() {
     // Return iterator to first element in collection
     _ptr = _forward ? _list->_first : _list->_last ;
@@ -309,18 +292,29 @@ class RooLinkedListIterImpl final : public TIterator {
   }
 
   bool operator!=(const RooLinkedListIterImpl &aIter) const {
-    return (_ptr != aIter._ptr);
+    return _ptr != aIter._ptr;
   }
 
   virtual TObject *operator*() const {
     // Return element iterator points to
-    return (_ptr ? _ptr->_arg : nullptr);
+    return _ptr ? _ptr->_arg : nullptr;
+  }
+
+  RooLinkedListIterImpl &operator++() {
+     if(_ptr) _ptr = _forward ? _ptr->_next : _ptr->_prev ;
+     return *this;
+  }
+
+  RooLinkedListIterImpl operator++(int) {
+     RooLinkedListIterImpl tmp(*this);
+     operator++();
+     return tmp;
   }
 
 protected:
   const RooLinkedList* _list ;     //! Collection iterated over
   const RooLinkedListElem* _ptr ;  //! Next link element
-  Bool_t _forward ;                //!  Iterator direction
+  bool _forward ;                  //!  Iterator direction
 };
 
 

roofit/roofitcore/src/RooCmdConfig.cxx

@@ -476,12 +476,9 @@ void RooCmdConfig::print()
 Bool_t RooCmdConfig::process(const RooLinkedList& argList) 
 {
   Bool_t ret(kFALSE) ;
-  TIterator* iter = argList.MakeIterator() ;
-  RooCmdArg* arg ;
-  while((arg=(RooCmdArg*)iter->Next())) {
+  for(auto * arg : static_range_cast<RooCmdArg*>(argList)) {
     ret |= process(*arg) ;
   }
-  delete iter ;
   return ret ;
 }
 

roofit/roofitcore/src/RooGenFitStudy.cxx

@@ -78,19 +78,8 @@ RooGenFitStudy::RooGenFitStudy(const RooGenFitStudy& other) :
   _params(0),
   _initParams(0)
 {  
-  TIterator* giter = other._genOpts.MakeIterator() ;
-  TObject* o ;
-  while((o=giter->Next())) {
-    _genOpts.Add(o->Clone()) ;
-  }
-  delete giter ;
-
-  TIterator* fiter = other._fitOpts.MakeIterator() ;
-  while((o=fiter->Next())) {
-    _fitOpts.Add(o->Clone()) ;
-  }
-  delete fiter ;
-
+  for(TObject * o : other._genOpts) _genOpts.Add(o->Clone());
+  for(TObject * o : other._fitOpts) _fitOpts.Add(o->Clone());
 }
 
 

roofit/roofitcore/src/RooLinkedList.cxx

@@ -752,6 +752,22 @@ RooFIter RooLinkedList::fwdIterator() const {
   return RooFIter(std::move(iterImpl));
 }
 
+RooLinkedListIterImpl RooLinkedList::begin() const {
+  return {this, true};
+}
+
+RooLinkedListIterImpl RooLinkedList::end() const {
+  return {this, nullptr, true};
+}
+
+RooLinkedListIterImpl RooLinkedList::rbegin() const {
+  return {this, false};
+}
+
+RooLinkedListIterImpl RooLinkedList::rend() const {
+  return {this, nullptr, false};
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 void RooLinkedList::Sort(Bool_t ascend) 

roofit/roofitcore/src/RooWorkspace.cxx

@@ -184,22 +184,14 @@ RooWorkspace::RooWorkspace(const RooWorkspace& other) :
   other._allOwnedNodes.snapshot(_allOwnedNodes,kTRUE) ;
 
   // Copy datasets
-  TIterator* iter = other._dataList.MakeIterator() ;
-  TObject* data2 ;
-  while((data2=iter->Next())) {
-    _dataList.Add(data2->Clone()) ;
-  }
-  delete iter ;
+  for(TObject *data2 : other._dataList) _dataList.Add(data2->Clone());
 
   // Copy snapshots
-  TIterator* iter2 = other._snapshots.MakeIterator() ;
-  RooArgSet* snap ;
-  while((snap=(RooArgSet*)iter2->Next())) {
-    RooArgSet* snapClone = (RooArgSet*) snap->snapshot() ;
+  for(auto * snap : static_range_cast<RooArgSet*>(other._snapshots)) {
+    auto snapClone = static_cast<RooArgSet*>(snap->snapshot());
     snapClone->setName(snap->GetName()) ;
     _snapshots.Add(snapClone) ;
   }
-  delete iter2 ;
 
   // Copy named sets
   for (map<string,RooArgSet>::const_iterator iter3 = other._namedSets.begin() ; iter3 != other._namedSets.end() ; ++iter3) {