ENH: relocate Foam::sort from PtrListOps to UPtrList.H

- can sort directly without ListOps or other intermediates
  (eg labelList order).

- PtrListOps::less/greater wrappers -> UPtrList::less/greater

applications/test/PtrList/Test-PtrList.C

@@ -621,10 +621,33 @@ int main(int argc, char *argv[])
         sortedOrder(dynPlanes2, order);
         Info<< "sorted order: " << flatOutput(order) << nl;
 
-        sortedOrder(dynPlanes2, order, PtrListOps::greater<plane>(dynPlanes2));
+        sortedOrder(dynPlanes2, order, UPtrList<plane>::greater(dynPlanes2));
         Info<< "sorted order: " << flatOutput(order) << nl;
 
+        // Shuffle
+        shuffle(dynPlanes2);
+        Info<< "Shuffled" << endl;
+        report(Info, dynPlanes2, false);
+
+        // Reverse sort
+        sort
+        (
+            dynPlanes2,
+            [](const plane& a, const plane& b) { return (b < a); }
+        );
+        Info<< "Reverse sorted" << endl;
+        report(Info, dynPlanes2, false);
+
+        // Forward sort
         sort(dynPlanes2);
+        Info<< "Sorted" << endl;
+        report(Info, dynPlanes2, false);
+
+        // Reverse pointer list - not yet available or needed!
+        /// reverse(dynPlanes2);
+        /// Info<< "Reversed" << endl;
+        /// report(Info, dynPlanes2, false);
+
         // dynPlanes2.squeezeNull();
 
         Info<< "Append" << endl;

src/OpenFOAM/containers/PtrLists/PtrListDetail/PtrListDetail.C

@@ -50,6 +50,9 @@ Foam::label Foam::Detail::PtrListDetail<T>::count() const
 template<class T>
 Foam::label Foam::Detail::PtrListDetail<T>::findNull() const
 {
+    // Same as  List<T*>::find(nullptr);
+    // except perhaps without pointer ambiguities...
+
     label idx = 0;
 
     for (const T* ptr : *this)

src/OpenFOAM/containers/PtrLists/PtrListDetail/PtrListDetail.H

@@ -42,8 +42,8 @@ SourceFiles
 
 \*---------------------------------------------------------------------------*/
 
-#ifndef PtrListDetail_H
+#ifndef Foam_PtrListDetail_H
-#define PtrListDetail_H
+#define Foam_PtrListDetail_H
 
 #include "List.H"
 
@@ -92,9 +92,12 @@ public:
         //- Return the count of non-nullptr entries
         label count() const;
 
-        //- Locate the first null entry, -1 if there are not any
+        //- Locate the first null entry, -1 if there are none (or empty list)
         label findNull() const;
 
+        //- FatalError if any null exists in the list
+        inline void checkNonNull() const;
+
         //- Assign all pointers to nullptr, without deleting.
         void setNull();
 

src/OpenFOAM/containers/PtrLists/PtrListDetail/PtrListDetailI.H

@@ -109,6 +109,20 @@ inline void Foam::Detail::PtrListDetail<T>::resize(const label newLen)
 }
 
 
+template<class T>
+inline void Foam::Detail::PtrListDetail<T>::checkNonNull() const
+{
+    const label idx = this->findNull();
+
+    if (idx >= 0)
+    {
+        FatalErrorInFunction
+            << "Element " << idx << " is null" << nl
+            << abort(FatalError);
+    }
+}
+
+
 // * * * * * * * * * * * * * * * Member Operators  * * * * * * * * * * * * * //
 
 template<class T>

src/OpenFOAM/containers/PtrLists/PtrListOps/PtrListOps.H

@@ -5,7 +5,7 @@
     \\  /    A nd           | www.openfoam.com
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
-    Copyright (C) 2019-2021 OpenCFD Ltd.
+    Copyright (C) 2019-2022 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -40,8 +40,8 @@ SourceFiles
 
 \*---------------------------------------------------------------------------*/
 
-#ifndef PtrListOps_H
+#ifndef Foam_PtrListOps_H
-#define PtrListOps_H
+#define Foam_PtrListOps_H
 
 #include "predicates.H"
 #include "PtrList.H"
@@ -54,31 +54,23 @@ namespace Foam
 
 //- Return (stable) sort order for the list
 template<class T>
-labelList sortedOrder(const UPtrList<T>& input);
+labelList sortedOrder(const UPtrList<T>& list);
 
 //- Generate (stable) sort order for the list
 template<class T>
-void sortedOrder(const UPtrList<T>& input, labelList& order);
+void sortedOrder(const UPtrList<T>& list, labelList& order);
 
 //- Generate (stable) sort order for the list,
 //- using the specified list compare predicate
 template<class T, class ListComparePredicate>
 void sortedOrder
 (
-    const UPtrList<T>& input,
+    const UPtrList<T>& list,
     labelList& order,
     const ListComparePredicate& comp
 );
 
 
-//- Inplace (stable) sorting of pointer list.
-template<class T>
-void sort(UPtrList<T>& list);
-
-//- Inplace (stable) sorting of pointer list.
-template<class T, class Compare>
-void sort(UPtrList<T>& list, const Compare& comp);
-
 //- Inplace shuffle of pointer list.
 template<class T>
 void shuffle(UPtrList<T>& list);
@@ -91,52 +83,6 @@ void shuffle(UPtrList<T>& list);
 namespace PtrListOps
 {
 
-// Public Classes
-
-//- A UPtrList compare binary predicate for normal sort.
-//  Null entries sort to the end
-template<class T>
-struct less
-{
-    const UPtrList<T>& values;
-
-    less(const UPtrList<T>& list)
-    :
-        values(list)
-    {}
-
-    bool operator()(const label a, const label b) const
-    {
-        const T* const ptr1 = values(a);
-        const T* const ptr2 = values(b);
-
-        return (ptr1 && ptr2) ? (*ptr1 < *ptr2) : !ptr2;
-    }
-};
-
-
-//- A UPtrList compare binary predicate for reverse sort.
-//  Null entries sort to the end
-template<class T>
-struct greater
-{
-    const UPtrList<T>& values;
-
-    greater(const UPtrList<T>& list)
-    :
-        values(list)
-    {}
-
-    bool operator()(const label a, const label b) const
-    {
-        const T* const ptr1 = values(a);
-        const T* const ptr2 = values(b);
-
-        return (ptr1 && ptr2) ? (*ptr2 < *ptr1) : !ptr2;
-    }
-};
-
-
 //- List of values generated by applying the access operation
 //- to each list item.
 //

src/OpenFOAM/containers/PtrLists/PtrListOps/PtrListOpsTemplates.C

@@ -5,7 +5,7 @@
     \\  /    A nd           | www.openfoam.com
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
-    Copyright (C) 2019-2021 OpenCFD Ltd.
+    Copyright (C) 2019-2022 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -32,11 +32,11 @@ License
 template<class T>
 Foam::labelList Foam::sortedOrder
 (
-    const UPtrList<T>& input
+    const UPtrList<T>& list
 )
 {
     labelList order;
-    sortedOrder(input, order, typename PtrListOps::less<T>(input));
+    Foam::sortedOrder(list, order, typename UPtrList<T>::less(list));
     return order;
 }
 
@@ -44,59 +44,49 @@ Foam::labelList Foam::sortedOrder
 template<class T>
 void Foam::sortedOrder
 (
-    const UPtrList<T>& input,
+    const UPtrList<T>& list,
     labelList& order
 )
 {
-    sortedOrder(input, order, typename PtrListOps::less<T>(input));
+    Foam::sortedOrder(list, order, typename UPtrList<T>::less(list));
 }
 
 
 template<class T, class ListComparePredicate>
 void Foam::sortedOrder
 (
-    const UPtrList<T>& input,
+    const UPtrList<T>& list,
     labelList& order,
     const ListComparePredicate& comp
 )
 {
     // List lengths must be identical. Old content is overwritten
-    order.resize_nocopy(input.size());
+    order.resize_nocopy(list.size());
 
-    ListOps::identity(order);
+    // Same as std::iota and ListOps::identity
-
+    label value = 0;
-    Foam::stableSort(order, comp);
+    for (label& item : order)
+    {
+        item = value;
+        ++value;
     }
 
-
+    std::stable_sort(order.begin(), order.end(), comp);
-template<class T>
-void Foam::sort(UPtrList<T>& list)
-{
-    labelList order;
-    sortedOrder(list, order);
-    list.sortOrder(order, false);  // false = allow nullptr
-}
-
-
-template<class T, class Compare>
-void Foam::sort(UPtrList<T>& list, const Compare& comp)
-{
-    labelList order;
-    sortedOrder(list, order, comp);
-    list.sortOrder(order, false);  // false = allow nullptr
 }
 
 
 template<class T>
 void Foam::shuffle(UPtrList<T>& list)
 {
+    // Cannot use std::shuffle directly since that would dereference
+    // the list entries, which may contain null pointers.
+    // The alternative would be to expose the pointer details (a bit ugly).
     labelList order(identity(list.size()));
     Foam::shuffle(order);
-    list.sortOrder(order, false);  // false = allow nullptr
+    list.sortOrder(order, false);  // false = no nullptr check
 }
 
 
-
 // Templated implementation for types(), names(), etc - file-scope
 template<class ReturnType, class T, class AccessOp>
 Foam::List<ReturnType> Foam::PtrListOps::get

src/OpenFOAM/containers/PtrLists/UPtrList/UPtrList.C

@@ -6,7 +6,7 @@
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
     Copyright (C) 2011-2016 OpenFOAM Foundation
-    Copyright (C) 2015-2019 OpenCFD Ltd.
+    Copyright (C) 2015-2022 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -66,11 +66,7 @@ Foam::label Foam::UPtrList<T>::squeezeNull()
 
 
 template<class T>
-void Foam::UPtrList<T>::reorder
+void Foam::UPtrList<T>::reorder(const labelUList& oldToNew, const bool check)
-(
-    const labelUList& oldToNew,
-    const bool testNull
-)
 {
     const label len = this->size();
 
@@ -87,37 +83,31 @@ void Foam::UPtrList<T>::reorder
 
     for (label i=0; i<len; ++i)
     {
-        const label idx = oldToNew[i];
+        const label newIdx = oldToNew[i];
 
-        if (idx < 0 || idx >= len)
+        if (newIdx < 0 || newIdx >= len)
         {
             FatalErrorInFunction
-                << "Illegal index " << idx << nl
+                << "Illegal index " << newIdx << nl
                 << "Valid indices are [0," << len << ") for type "
                 << typeid(T).name() << nl
                 << abort(FatalError);
         }
 
-        if (newList[idx])
+        if (newList[newIdx])
         {
             FatalErrorInFunction
-                << "reorder map is not unique; element " << idx
+                << "reorder map is not unique; element " << newIdx
                 << " already used for type " << typeid(T).name()
                 << abort(FatalError);
         }
-        newList[idx] = ptrs_[i];
+        newList[newIdx] = ptrs_[i];
     }
 
-    // Verify that all pointers were indeed set
+    // Verify all pointers were indeed set
-    if (testNull)
+    if (check)
     {
-        const label idx = newList.findNull();
+        newList.checkNonNull();
-        if (idx >= 0)
-        {
-            FatalErrorInFunction
-                << "Element " << idx << " not set after reordering." << nl
-                << abort(FatalError);
-        }
     }
 
     ptrs_.transfer(newList);
@@ -125,11 +115,7 @@ void Foam::UPtrList<T>::reorder
 
 
 template<class T>
-void Foam::UPtrList<T>::sortOrder
+void Foam::UPtrList<T>::sortOrder(const labelUList& order, const bool check)
-(
-    const labelUList& order,
-    const bool testNull
-)
 {
     const label len = this->size();
 
@@ -147,39 +133,33 @@ void Foam::UPtrList<T>::sortOrder
 
     for (label i=0; i<len; ++i)
     {
-        const label idx = order[i];
+        const label oldIdx = order[i];
 
-        if (idx < 0 || idx >= len)
+        if (oldIdx < 0 || oldIdx >= len)
         {
             FatalErrorInFunction
-                << "Illegal index " << idx << nl
+                << "Illegal index " << oldIdx << nl
                 << "Valid indices are [0," << len << ") for type "
                 << typeid(T).name() << nl
                 << abort(FatalError);
         }
 
-        if (guard[idx])
+        if (guard[oldIdx])
         {
             FatalErrorInFunction
-                << "order map is not unique; element " << idx
+                << "order map is not unique; element " << oldIdx
                 << " already used for type " << typeid(T).name()
                 << abort(FatalError);
         }
 
-        guard[idx] = ptrs_[idx];
+        guard[oldIdx] = ptrs_[oldIdx];
-        newList[i] = ptrs_[idx];
+        newList[i] = ptrs_[oldIdx];
     }
 
     // Verify that all pointers were indeed set
-    if (testNull)
+    if (check)
     {
-        const label idx = newList.findNull();
+        newList.checkNonNull();
-        if (idx >= 0)
-        {
-            FatalErrorInFunction
-                << "Element " << idx << " not set after reordering." << nl
-                << abort(FatalError);
-        }
     }
 
     ptrs_.transfer(newList);
@@ -195,4 +175,26 @@ Foam::Ostream& Foam::operator<<(Ostream& os, const UPtrList<T>& list)
 }
 
 
+// * * * * * * * * * * * * * * * Global Functions  * * * * * * * * * * * * * //
+
+template<class T, class Compare>
+void Foam::sort(UPtrList<T>& list, const Compare& comp)
+{
+    std::stable_sort
+    (
+        list.begin_ptr(),
+        list.end_ptr(),
+        typename UPtrList<T>::template value_compare<Compare>(comp)
+    );
+}
+
+
+template<class T>
+void Foam::sort(UPtrList<T>& list)
+{
+    // ie, lessOp<T>() or std::less<T>()
+    Foam::sort(list, [](const T& a, const T& b) { return (a < b); });
+}
+
+
 // ************************************************************************* //

src/OpenFOAM/containers/PtrLists/UPtrList/UPtrList.H

@@ -110,6 +110,72 @@ public:
         class const_iterator;
 
 
+    // Public Classes
+
+        //- A wrapper for a binary comparison of values that interjects
+        //- pointer dereferencing with null pointer guards.
+        //  It will also sort any null pointers to the end
+        //  (eg, rubbish that can be truncated)
+        template<class Compare>
+        struct value_compare
+        {
+            const Compare& comp;
+
+            value_compare(const Compare& cmp)
+            :
+                comp(cmp)
+            {}
+
+            //- Compare dereferenced pointers
+            bool operator()(const T* const a, const T* const b) const
+            {
+                return (a && b) ? comp(*a, *b) : bool(a);
+            }
+        };
+
+        //- A UPtrList compare binary predicate for normal sort order.
+        //- Null entries (if any) sort to the end.
+        struct less
+        {
+            const UPtrList<T>& values;
+
+            less(const UPtrList<T>& list)
+            :
+                values(list)
+            {}
+
+            //- Compare dereferenced pointer locations for normal sort.
+            bool operator()(const label ai, const label bi) const
+            {
+                const T* const a = values.get(ai);
+                const T* const b = values.get(bi);
+
+                return (a && b) ? (*a < *b) : bool(a);
+            }
+        };
+
+        //- A UPtrList compare binary predicate for reverse sort order.
+        //  Null entries (if any) sort to the end.
+        struct greater
+        {
+            const UPtrList<T>& values;
+
+            greater(const UPtrList<T>& list)
+            :
+                values(list)
+            {}
+
+            //- Compare dereferenced pointer locations for reverse sort
+            bool operator()(const label ai, const label bi) const
+            {
+                const T* const a = values.get(ai);
+                const T* const b = values.get(bi);
+
+                return (a && b) ? (*b < *a) : bool(b);
+            }
+        };
+
+
     // Constructors
 
         //- Default construct
@@ -215,13 +281,19 @@ public:
 
         //- Reorder elements.
         //- Reordering must be unique (ie, shuffle).
-        //  Optionally verify that all pointers have been set.
+        //  Optionally check that all pointers have been set.
-        void reorder(const labelUList& oldToNew, const bool testNull = true);
+        void reorder(const labelUList& oldToNew, const bool check = false);
 
         //- Reorder elements according to new order mapping (newToOld).
         //- Reordering must be unique (ie, shuffle).
-        //  Optionally verify that all pointers have been set.
+        //  Optionally check that all pointers have been set.
-        void sortOrder(const labelUList& order, const bool testNull = true);
+        void sortOrder(const labelUList& order, const bool check = false);
+
+
+    // Checks
+
+        //- Check and raise FatalError if any nullptr exists in the list
+        inline void checkNonNull() const;
 
 
     // Member Operators
@@ -362,22 +434,29 @@ public:
         };
 
 
-        //- Return an iterator to begin traversing the UPtrList
+        //- Iterator to begin of raw pointers traversal (use with caution)
+        T** begin_ptr() noexcept { return ptrs_.begin(); }
+
+        //- Iterator beyond end of raw pointers traversal (use with caution)
+        T** end_ptr() noexcept { return ptrs_.end(); }
+
+
+        //- Return an iterator to begin of UPtrList traversal
         inline iterator begin() noexcept;
 
-        //- Return an iterator to end traversing the UPtrList
+        //- Return iterator beyond end of UPtrList traversal
         inline iterator end() noexcept;
 
-        //- Return an const_iterator to begin traversing the UPtrList
+        //- Return const_iterator to begin of UPtrList traversal
         inline const_iterator cbegin() const noexcept;
 
-        //- Return an const_iterator to end traversing the UPtrList
+        //- Return const_iterator beyond end of UPtrList traversal
         inline const_iterator cend() const noexcept;
 
-        //- Return an const_iterator to begin traversing the UPtrList
+        //- Return const_iterator to begin of UPtrList traversal
         inline const_iterator begin() const noexcept;
 
-        //- Return an const_iterator to end traversing the UPtrList
+        //- Return const_iterator beyond end of UPtrList traversal
         inline const_iterator end() const noexcept;
 
 
@@ -392,6 +471,20 @@ public:
 };
 
 
+// * * * * * * * * * * * * * * Global Functions  * * * * * * * * * * * * * * //
+
+//- Inplace (stable) sorting of pointer list.
+template<class T>
+void sort(UPtrList<T>& list);
+
+//- Inplace (stable) sorting of pointer list using given comparator,
+//- which compares objects, not pointers.
+//  This sort function includes null pointer guards and will also sort
+//  any null pointers to the end (eg, rubbish that can be truncated)
+template<class T, class Compare>
+void sort(UPtrList<T>& list, const Compare& comp);
+
+
 // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
 
 } // End namespace Foam

src/OpenFOAM/containers/PtrLists/UPtrList/UPtrListI.H

@@ -215,6 +215,13 @@ inline T* Foam::UPtrList<T>::set(const label i, T* ptr)
 }
 
 
+template<class T>
+inline void Foam::UPtrList<T>::checkNonNull() const
+{
+    ptrs_.checkNonNull();
+}
+
+
 // * * * * * * * * * * * * * * * Member Operators  * * * * * * * * * * * * * //
 
 template<class T>