From 42fe95a6a96abd2d3a8c12d4b3d90c6f6f9aeb1a Mon Sep 17 00:00:00 2001 From: Mark Olesen Date: Mon, 7 Mar 2022 10:46:25 +0100 Subject: [PATCH] ENH: minor de-clutter of List, DynamicList, DynamicField (#2385) - do not need contruct or move assign from SortableList. Rarely (never) used and can simply treat like a normal list by applying shrink beforehand. - make append() methods return void instead of returning self, which makes it easier to derive from. Having them return self was a bit of an original design mistake. Chaining appends do not actually occur anywhere. Even if they were to be used, would not want to rely on them (fear of slicing on any derived classes). BUG: IndirectList iterator comparison loses constness --- .../test/IndirectList/Test-IndirectList.C | 12 +++ .../containers/Bits/PackedList/PackedList.H | 9 +- .../IndirectListBase/IndirectListBase.H | 62 ++++++------- .../Lists/DynamicList/DynamicList.H | 45 +++------- .../Lists/DynamicList/DynamicListI.H | 90 +++---------------- src/OpenFOAM/containers/Lists/List/List.C | 27 +----- src/OpenFOAM/containers/Lists/List/List.H | 17 +--- .../Lists/SortableList/SortableList.C | 4 +- .../Lists/SortableList/SortableList.H | 24 ++--- .../fields/Fields/DynamicField/DynamicField.H | 13 ++- .../Fields/DynamicField/DynamicFieldI.H | 14 +-- 11 files changed, 91 insertions(+), 226 deletions(-) diff --git a/applications/test/IndirectList/Test-IndirectList.C b/applications/test/IndirectList/Test-IndirectList.C index 2f3a2e45d4..27736a2cd6 100644 --- a/applications/test/IndirectList/Test-IndirectList.C +++ b/applications/test/IndirectList/Test-IndirectList.C @@ -106,6 +106,18 @@ int main(int argc, char *argv[]) testFind(val, idl1); } + { + auto iter = idl1.cbegin(); + const auto endIter = idl1.cend(); + + while (iter != endIter) + { + // No post-fix increment: + Info<< *iter << nl; + ++iter; + } + } + inplaceReverseList(addresses); idl1.addressing() = std::move(addresses); diff --git a/src/OpenFOAM/containers/Bits/PackedList/PackedList.H b/src/OpenFOAM/containers/Bits/PackedList/PackedList.H index 23401256b6..1cb1395ad2 100644 --- a/src/OpenFOAM/containers/Bits/PackedList/PackedList.H +++ b/src/OpenFOAM/containers/Bits/PackedList/PackedList.H @@ -89,8 +89,8 @@ SourceFiles \*---------------------------------------------------------------------------*/ -#ifndef PackedList_H -#define PackedList_H +#ifndef Foam_PackedList_H +#define Foam_PackedList_H #include "BitOps.H" #include "labelList.H" @@ -362,10 +362,9 @@ public: //- Currently identical to resize. Subject to future change (Oct-2021) inline void resize_nocopy(const label numElem); - //- Reserve allocation space for at least this size. + //- Reserve allocation space for at least this size + //- (uses a size doubling strategy). // Never shrinks the allocated size. - // The list size is adjusted as per DynamicList with - // SizeInc=0, SizeMult=2, SizeDiv=1 inline void reserve(const label numElem); //- Clear the list, i.e. set addressable size to zero. diff --git a/src/OpenFOAM/containers/IndirectLists/IndirectListBase/IndirectListBase.H b/src/OpenFOAM/containers/IndirectLists/IndirectListBase/IndirectListBase.H index ae54058005..2c01a2c75c 100644 --- a/src/OpenFOAM/containers/IndirectLists/IndirectListBase/IndirectListBase.H +++ b/src/OpenFOAM/containers/IndirectLists/IndirectListBase/IndirectListBase.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2017-2021 OpenCFD Ltd. + Copyright (C) 2017-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -39,8 +39,8 @@ SourceFiles \*---------------------------------------------------------------------------*/ -#ifndef IndirectListBase_H -#define IndirectListBase_H +#ifndef Foam_IndirectListBase_H +#define Foam_IndirectListBase_H #include "List.H" @@ -242,9 +242,11 @@ public: // Iterators //- A non-const iterator for an indirect list + // Only supports forward prefix increment, since the addressing + // may/may not support postfix or decrement. class iterator { - typename UList::pointer data_; + T* begin_; typename addressing_type::const_iterator iter_; public: @@ -258,17 +260,14 @@ public: iterator ( UList& list, - typename addressing_type::const_iterator baseIter + typename addressing_type::const_iterator addrIter ) : - data_(list.begin()), - iter_(baseIter) + begin_(list.data()), + iter_(addrIter) {} - reference operator*() const - { - return data_[*iter_]; - } + reference operator*() const { return *(begin_ + *iter_); } iterator& operator++() { @@ -276,12 +275,12 @@ public: return *this; } - bool operator==(iterator& rhs) const + bool operator==(const iterator& rhs) const { - return iter_ == rhs.iter_; + return (iter_ == rhs.iter_); } - bool operator!=(iterator& rhs) const + bool operator!=(const iterator& rhs) const { return (iter_ != rhs.iter_); } @@ -289,9 +288,11 @@ public: //- A const iterator for an indirect list + // Only supports forward prefix increment, since the addressing + // may/may not support postfix or decrement. class const_iterator { - typename UList::const_pointer data_; + const T* begin_; typename addressing_type::const_iterator iter_; public: @@ -305,17 +306,14 @@ public: const_iterator ( const UList& list, - typename addressing_type::const_iterator baseIter + typename addressing_type::const_iterator addrIter ) : - data_(list.begin()), - iter_(baseIter) + begin_(list.cdata()), + iter_(addrIter) {} - reference operator*() const - { - return data_[*iter_]; - } + reference operator*() const { return *(begin_ + *iter_); } const_iterator& operator++() { @@ -323,14 +321,14 @@ public: return *this; } - bool operator==(const_iterator& rhs) const + bool operator==(const const_iterator& rhs) const { - return iter_ == rhs.iter_; + return (iter_ == rhs.iter_); } - bool operator!=(const_iterator& rhs) const + bool operator!=(const const_iterator& rhs) const { - return iter_ != rhs.iter_; + return (iter_ != rhs.iter_); } }; @@ -364,17 +362,11 @@ public: return const_iterator(values_, addr_.cend()); } - //- Return a const_iterator at end of list - inline const_iterator begin() const - { - return cbegin(); - } + //- Return a const_iterator at begin of list + const_iterator begin() const { return cbegin(); } //- Return a const_iterator at end of list - inline const_iterator end() const - { - return cend(); - } + const_iterator end() const { return cend(); } // Writing diff --git a/src/OpenFOAM/containers/Lists/DynamicList/DynamicList.H b/src/OpenFOAM/containers/Lists/DynamicList/DynamicList.H index c97645e21f..57919c75bf 100644 --- a/src/OpenFOAM/containers/Lists/DynamicList/DynamicList.H +++ b/src/OpenFOAM/containers/Lists/DynamicList/DynamicList.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2016-2021 OpenCFD Ltd. + Copyright (C) 2016-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -42,8 +42,8 @@ SourceFiles \*---------------------------------------------------------------------------*/ -#ifndef DynamicList_H -#define DynamicList_H +#ifndef Foam_DynamicList_H +#define Foam_DynamicList_H #include "List.H" @@ -154,9 +154,6 @@ public: //- Move construct from List inline DynamicList(List&& lst); - //- Move construct from SortableList - DynamicList(SortableList&& lst); - //- Construct from Istream. Size set to size of list read. explicit DynamicList(Istream& is); @@ -259,47 +256,32 @@ public: template inline void transfer(DynamicList& list); - //- Transfer contents of the argument SortableList into this. - inline void transfer(SortableList& list); - - //- Append an element to the end of this list. - inline DynamicList& append(const T& val); + //- Copy append an element to the end of this list. + inline void append(const T& val); //- Move append an element - inline DynamicList& append(T&& val); + inline void append(T&& val); //- Append another list to the end of this list. - inline DynamicList& append(const UList& lst); + inline void append(const UList& lst); //- Append a FixedList to the end of this list. template - inline DynamicList& - append(const FixedList& lst); + inline void append(const FixedList& lst); //- Append an initializer list at the end of this list. - inline DynamicList& - append(std::initializer_list lst); + inline void append(std::initializer_list lst); //- Append a IndirectList at the end of this list template - inline DynamicList& - append(const IndirectListBase& lst); + inline void append(const IndirectListBase& lst); //- Move append list - inline DynamicList& append(List&& lst); - - //- Move append list - inline DynamicList& - append(DynamicList&& lst); + inline void append(List&& lst); //- Move append list template - inline DynamicList& - append(DynamicList&& lst); - - //- Move append list - inline DynamicList& - append(SortableList&& lst); + inline void append(DynamicList&& list); //- Append an element if not already in the list. // \return the change in list length @@ -377,9 +359,6 @@ public: template inline void operator=(DynamicList&& lst); - //- Move assignment - inline void operator=(SortableList&& lst); - // Reading/writing diff --git a/src/OpenFOAM/containers/Lists/DynamicList/DynamicListI.H b/src/OpenFOAM/containers/Lists/DynamicList/DynamicListI.H index 972317b735..ee52e49b84 100644 --- a/src/OpenFOAM/containers/Lists/DynamicList/DynamicListI.H +++ b/src/OpenFOAM/containers/Lists/DynamicList/DynamicListI.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2016-2021 OpenCFD Ltd. + Copyright (C) 2016-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -493,21 +493,7 @@ Foam::DynamicList::transfer template -inline void -Foam::DynamicList::transfer -( - SortableList& list -) -{ - list.shrink(); // Shrink away sort indices - capacity_ = list.size(); // Capacity after transfer == list size - List::transfer(list); -} - - -template -inline Foam::DynamicList& -Foam::DynamicList::append +inline void Foam::DynamicList::append ( const T& val ) @@ -516,13 +502,11 @@ Foam::DynamicList::append resize(idx + 1); this->operator[](idx) = val; // copy element - return *this; } template -inline Foam::DynamicList& -Foam::DynamicList::append +inline void Foam::DynamicList::append ( T&& val ) @@ -531,13 +515,11 @@ Foam::DynamicList::append resize(idx + 1); this->operator[](idx) = std::move(val); // move assign element - return *this; } template -inline Foam::DynamicList& -Foam::DynamicList::append +inline void Foam::DynamicList::append ( const UList& lst ) @@ -556,14 +538,12 @@ Foam::DynamicList::append { this->operator[](idx++) = val; // copy element } - return *this; } template template -inline Foam::DynamicList& -Foam::DynamicList::append +inline void Foam::DynamicList::append ( const FixedList& lst ) @@ -575,13 +555,11 @@ Foam::DynamicList::append { this->operator[](idx++) = val; // copy element } - return *this; } template -inline Foam::DynamicList& -Foam::DynamicList::append +inline void Foam::DynamicList::append ( std::initializer_list lst ) @@ -593,14 +571,12 @@ Foam::DynamicList::append { this->operator[](idx++) = val; // copy element } - return *this; } template template -inline Foam::DynamicList& -Foam::DynamicList::append +inline void Foam::DynamicList::append ( const IndirectListBase& lst ) @@ -614,13 +590,11 @@ Foam::DynamicList::append { this->operator[](idx++) = lst[i]; // copy element } - return *this; } template -inline Foam::DynamicList& -Foam::DynamicList::append +inline void Foam::DynamicList::append ( List&& list ) @@ -641,47 +615,18 @@ Foam::DynamicList::append } list.clear(); - return *this; -} - - -template -inline Foam::DynamicList& -Foam::DynamicList::append -( - DynamicList&& lst -) -{ - append(std::move(static_cast&>(lst))); - lst.clearStorage(); // Ensure capacity=0 - return *this; } template template -inline Foam::DynamicList& -Foam::DynamicList::append +inline void Foam::DynamicList::append ( - DynamicList&& lst + DynamicList&& list ) { - append(std::move(static_cast&>(lst))); - lst.clearStorage(); // Ensure capacity=0 - return *this; -} - - -template -inline Foam::DynamicList& -Foam::DynamicList::append -( - SortableList&& lst -) -{ - lst.shrink(); // Shrink away sort indices - append(std::move(static_cast&>(lst))); - return *this; + append(std::move(static_cast&>(list))); + list.clearStorage(); // Ensure capacity=0 } @@ -939,15 +884,4 @@ inline void Foam::DynamicList::operator= } -template -inline void Foam::DynamicList::operator= -( - SortableList&& lst -) -{ - clear(); - transfer(lst); -} - - // ************************************************************************* // diff --git a/src/OpenFOAM/containers/Lists/List/List.C b/src/OpenFOAM/containers/Lists/List/List.C index d6bcf0cea1..0384aa013c 100644 --- a/src/OpenFOAM/containers/Lists/List/List.C +++ b/src/OpenFOAM/containers/Lists/List/List.C @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2017-2021 OpenCFD Ltd. + Copyright (C) 2017-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -405,15 +405,6 @@ Foam::List::List(DynamicList&& list) } -template -Foam::List::List(SortableList&& list) -: - UList() -{ - transfer(list); -} - - template Foam::List::List(SLList&& list) : @@ -483,15 +474,6 @@ void Foam::List::transfer(DynamicList& list) } -template -void Foam::List::transfer(SortableList& list) -{ - // Shrink away the sort indices - list.shrink(); - transfer(static_cast&>(list)); -} - - // * * * * * * * * * * * * * * * Member Operators * * * * * * * * * * * * * // template @@ -638,13 +620,6 @@ void Foam::List::operator=(DynamicList&& list) } -template -void Foam::List::operator=(SortableList&& list) -{ - transfer(list); -} - - template void Foam::List::operator=(SLList&& list) { diff --git a/src/OpenFOAM/containers/Lists/List/List.H b/src/OpenFOAM/containers/Lists/List/List.H index c300941d48..6bf34de25b 100644 --- a/src/OpenFOAM/containers/Lists/List/List.H +++ b/src/OpenFOAM/containers/Lists/List/List.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2017-2021 OpenCFD Ltd. + Copyright (C) 2017-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -40,8 +40,8 @@ SourceFiles \*---------------------------------------------------------------------------*/ -#ifndef List_H -#define List_H +#ifndef Foam_List_H +#define Foam_List_H #include "autoPtr.H" #include "UList.H" @@ -58,7 +58,6 @@ template class FixedList; template class DynamicList; template class PtrList; -template class SortableList; template Istream& operator>>(Istream& is, List& list); @@ -182,9 +181,6 @@ public: template List(DynamicList&& list); - //- Move construct from SortableList - List(SortableList&& list); - //- Move construct from SLList List(SLList&& list); @@ -258,10 +254,6 @@ public: template void transfer(DynamicList& list); - //- Transfer the contents of the argument List into this list - //- and annul the argument list - void transfer(SortableList& list); - //- Return subscript-checked element of UList and resizing the list //- if required. inline T& newElmt(const label i); @@ -302,9 +294,6 @@ public: template void operator=(DynamicList&& list); - //- Move assignment. Takes constant time. - void operator=(SortableList&& list); - //- Move assignment. Takes constant time void operator=(SLList&& list); diff --git a/src/OpenFOAM/containers/Lists/SortableList/SortableList.C b/src/OpenFOAM/containers/Lists/SortableList/SortableList.C index 70cc75a575..28288a5bc0 100644 --- a/src/OpenFOAM/containers/Lists/SortableList/SortableList.C +++ b/src/OpenFOAM/containers/Lists/SortableList/SortableList.C @@ -105,7 +105,7 @@ inline Foam::SortableList::SortableList(std::initializer_list values) // * * * * * * * * * * * * * * * Member Functions * * * * * * * * * * * * * // template -void Foam::SortableList::clear() +inline void Foam::SortableList::clear() { List::clear(); indices_.clear(); @@ -113,7 +113,7 @@ void Foam::SortableList::clear() template -Foam::List& Foam::SortableList::shrink() +inline Foam::List& Foam::SortableList::shrink() { indices_.clear(); return static_cast&>(*this); diff --git a/src/OpenFOAM/containers/Lists/SortableList/SortableList.H b/src/OpenFOAM/containers/Lists/SortableList/SortableList.H index 8a0ce95f02..3009fbeeef 100644 --- a/src/OpenFOAM/containers/Lists/SortableList/SortableList.H +++ b/src/OpenFOAM/containers/Lists/SortableList/SortableList.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2017-2021 OpenCFD Ltd. + Copyright (C) 2017-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -43,8 +43,8 @@ SourceFiles \*---------------------------------------------------------------------------*/ -#ifndef SortableList_H -#define SortableList_H +#ifndef Foam_SortableList_H +#define Foam_SortableList_H #include "labelList.H" @@ -106,22 +106,22 @@ public: // Member Functions //- Return the list of sorted indices. Updated every sort - const labelList& indices() const + const labelList& indices() const noexcept { return indices_; } //- Return non-const access to the sorted indices. Updated every sort - labelList& indices() + labelList& indices() noexcept { return indices_; } //- Clear the list and the indices - void clear(); + inline void clear(); //- Clear the indices and return a reference to the underlying List - List& shrink(); + inline List& shrink(); //- Forward (stable) sort the list (if changed after construction). // Resizes the indices as required @@ -163,15 +163,7 @@ public: }; -// * * * * * * * * * * * * * * * Global Functions * * * * * * * * * * * * * // - -// Exchange contents of lists - see SortableList::swap(). -template -inline void Swap(SortableList& a, SortableList& b) -{ - a.swap(b); -} - +// Note: uses default Foam::Swap (move construct/assignment) // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * // diff --git a/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.H b/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.H index 1293335420..1ae483354c 100644 --- a/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.H +++ b/src/OpenFOAM/fields/Fields/DynamicField/DynamicField.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2016-2021 OpenCFD Ltd. + Copyright (C) 2016-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -35,8 +35,8 @@ SourceFiles \*---------------------------------------------------------------------------*/ -#ifndef DynamicField_H -#define DynamicField_H +#ifndef Foam_DynamicField_H +#define Foam_DynamicField_H #include "Field.H" #include "DynamicList.H" @@ -283,14 +283,13 @@ public: inline void transfer(DynamicField& list); //- Append an element at the end of the list - inline DynamicField& append(const T& val); + inline void append(const T& val); //- Move append an element - inline DynamicField& append(T&& val); + inline void append(T&& val); //- Append a List at the end of this list - inline DynamicField& - append(const UList& list); + inline void append(const UList& list); //- Remove and return the top element inline T remove(); diff --git a/src/OpenFOAM/fields/Fields/DynamicField/DynamicFieldI.H b/src/OpenFOAM/fields/Fields/DynamicField/DynamicFieldI.H index 7a3da86416..bfca3759ca 100644 --- a/src/OpenFOAM/fields/Fields/DynamicField/DynamicFieldI.H +++ b/src/OpenFOAM/fields/Fields/DynamicField/DynamicFieldI.H @@ -6,7 +6,7 @@ \\/ M anipulation | ------------------------------------------------------------------------------- Copyright (C) 2011-2016 OpenFOAM Foundation - Copyright (C) 2016-2021 OpenCFD Ltd. + Copyright (C) 2016-2022 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -582,8 +582,7 @@ inline void Foam::DynamicField::transfer template -inline Foam::DynamicField& -Foam::DynamicField::append +inline void Foam::DynamicField::append ( const T& val ) @@ -592,13 +591,11 @@ Foam::DynamicField::append resize(idx + 1); this->operator[](idx) = val; // copy element - return *this; } template -inline Foam::DynamicField& -Foam::DynamicField::append +inline void Foam::DynamicField::append ( T&& val ) @@ -607,13 +604,11 @@ Foam::DynamicField::append resize(idx + 1); this->operator[](idx) = std::move(val); // move assign element - return *this; } template -inline Foam::DynamicField& -Foam::DynamicField::append +inline void Foam::DynamicField::append ( const UList& list ) @@ -632,7 +627,6 @@ Foam::DynamicField::append { this->operator[](idx++) = val; // copy element } - return *this; }