Commit c0a50dc6 authored by Mark OLESEN's avatar Mark OLESEN
Browse files

ENH: improve overall consistency of the HashTable and its iterators

- previously had a mismash of const/non-const attributes on iterators
  that were confused with the attributes of the object being accessed.

- use the iterator keys() and object() methods consistently for all
  internal access of the HashTable iterators. This makes the intention
  clearer, the code easier to maintain, and protects against any
  possible changes in the definition of the operators.

- 'operator*': The standard form expected by STL libraries.
  However, for the std::map, this dereferences to a <key,value> pair,
  whereas OpenFOAM dereferences simply to <value>.

- 'operator()': OpenFOAM treats this like the 'operator*'

- adjusted the values of end() and cend() to reinterpret from nullObject
  instead of returning a static iteratorEnd() object.
  This means that C++ templates can now correctly deduce and match
  the return types from begin() and end() consistently.
  So that range-based now works.

  Eg,
      HashTable<label> table1 = ...;
      for (auto i : table1)
      {
          Info<< i << endl;
      }

  Since the 'operator*' returns hash table values, this prints all the
  values in the table.
parent 0298c02b
......@@ -177,6 +177,12 @@ int main()
Info<< "\ntable1" << table1 << nl;
Info<< "\nrange-for(table1)" << nl;
for (auto const& it : table1)
{
Info<< " " << it << nl;
}
Info<< "\nDone\n";
return 0;
......
......@@ -45,7 +45,7 @@ Foam::HashPtrTable<T, Key, Hash>::HashPtrTable
{
for (const_iterator iter = ht.begin(); iter != ht.end(); ++iter)
{
const T* ptr = *iter;
const T* ptr = iter.object();
if (ptr)
{
this->insert(iter.key(), new T(*ptr));
......@@ -72,7 +72,7 @@ Foam::HashPtrTable<T, Key, Hash>::~HashPtrTable()
template<class T, class Key, class Hash>
T* Foam::HashPtrTable<T, Key, Hash>::remove(iterator& iter)
{
T* ptr = *iter;
T* ptr = iter.object();
HashTable<T*, Key, Hash>::erase(iter);
return ptr;
}
......@@ -81,7 +81,7 @@ T* Foam::HashPtrTable<T, Key, Hash>::remove(iterator& iter)
template<class T, class Key, class Hash>
bool Foam::HashPtrTable<T, Key, Hash>::erase(iterator& iter)
{
T* ptr = *iter;
T* ptr = iter.object();
if (HashTable<T*, Key, Hash>::erase(iter))
{
......@@ -102,14 +102,9 @@ bool Foam::HashPtrTable<T, Key, Hash>::erase(iterator& iter)
template<class T, class Key, class Hash>
void Foam::HashPtrTable<T, Key, Hash>::clear()
{
for
(
iterator iter = this->begin();
iter != this->end();
++iter
)
for (iterator iter = this->begin(); iter != this->end(); ++iter)
{
delete *iter;
delete iter.object();
}
HashTable<T*, Key, Hash>::clear();
......@@ -136,7 +131,7 @@ void Foam::HashPtrTable<T, Key, Hash>::operator=
for (const_iterator iter = rhs.begin(); iter != rhs.end(); ++iter)
{
const T* ptr = *iter;
const T* ptr = iter.object();
if (ptr)
{
this->insert(iter.key(), new T(*ptr));
......
......@@ -54,7 +54,7 @@ template<class T, class Key, class Hash>
Istream& operator>>(Istream& is, HashPtrTable<T, Key, Hash>& L);
template<class T, class Key, class Hash>
Ostream& operator<<(Ostream& os, const HashPtrTable<T, Key, Hash>& L);
Ostream& operator<<(Ostream& os, const HashPtrTable<T, Key, Hash>& tbl);
/*---------------------------------------------------------------------------*\
......@@ -80,8 +80,8 @@ class HashPtrTable
public:
typedef typename HashTable<T*, Key, Hash>::iterator iterator;
typedef typename HashTable<T*, Key, Hash>::const_iterator const_iterator;
using iterator = typename HashTable<T*, Key, Hash>::iterator;
using const_iterator = typename HashTable<T*, Key, Hash>::const_iterator;
// Constructors
......@@ -141,7 +141,7 @@ public:
friend Ostream& operator<< <T, Key, Hash>
(
Ostream& os,
const HashPtrTable<T, Key, Hash>& L
const HashPtrTable<T, Key, Hash>& tbl
);
};
......
......@@ -157,16 +157,9 @@ void Foam::HashPtrTable<T, Key, Hash>::read
template<class T, class Key, class Hash>
void Foam::HashPtrTable<T, Key, Hash>::write(Ostream& os) const
{
for
(
typename HashPtrTable<T, Key, Hash>::const_iterator
iter = this->begin();
iter != this->end();
++iter
)
for (const_iterator iter = this->begin(); iter != this->end(); ++iter)
{
const T* ptr = *iter;
const T* ptr = iter.object();
if (ptr)
{
ptr->write(os);
......@@ -215,21 +208,18 @@ template<class T, class Key, class Hash>
Foam::Ostream& Foam::operator<<
(
Ostream& os,
const HashPtrTable<T, Key, Hash>& L
const HashPtrTable<T, Key, Hash>& tbl
)
{
using const_iterator = typename HashPtrTable<T, Key, Hash>::const_iterator;
// Write size and start delimiter
os << nl << L.size() << nl << token::BEGIN_LIST << nl;
os << nl << tbl.size() << nl << token::BEGIN_LIST << nl;
// Write contents
for
(
typename HashPtrTable<T, Key, Hash>::const_iterator iter = L.begin();
iter != L.end();
++iter
)
for (const_iterator iter = tbl.cbegin(); iter != tbl.cend(); ++iter)
{
const T* ptr = *iter;
const T* ptr = iter.object();
os << iter.key();
if (ptr)
......
......@@ -117,20 +117,17 @@ template<class Key, class Hash>
template<class AnyType, class AnyHash>
Foam::HashSet<Key, Hash>::HashSet
(
const HashTable<AnyType, Key, AnyHash>& h
const HashTable<AnyType, Key, AnyHash>& tbl
)
:
HashTable<nil, Key, Hash>(h.size())
HashTable<nil, Key, Hash>(tbl.size())
{
for
(
typename HashTable<AnyType, Key, AnyHash>::const_iterator
cit = h.cbegin();
cit != h.cend();
++cit
)
using other_iter =
typename HashTable<AnyType, Key, AnyHash>::const_iterator;
for (other_iter iter = tbl.cbegin(); iter != tbl.cend(); ++iter)
{
this->insert(cit.key());
this->insert(iter.key());
}
}
......
......@@ -132,12 +132,12 @@ public:
//- Construct from the keys of another HashTable,
// the type of values held is arbitrary.
template<class AnyType, class AnyHash>
explicit HashSet(const HashTable<AnyType, Key, AnyHash>& h);
explicit HashSet(const HashTable<AnyType, Key, AnyHash>& tbl);
// Member Functions
// Edit
// Edit
//- Insert a new entry
bool insert(const Key& key)
......
......@@ -76,7 +76,7 @@ Foam::HashTable<T, Key, Hash>::HashTable(const label size)
for (label hashIdx = 0; hashIdx < tableSize_; hashIdx++)
{
table_[hashIdx] = 0;
table_[hashIdx] = nullptr;
}
}
}
......@@ -89,7 +89,7 @@ Foam::HashTable<T, Key, Hash>::HashTable(const HashTable<T, Key, Hash>& ht)
{
for (const_iterator iter = ht.cbegin(); iter != ht.cend(); ++iter)
{
insert(iter.key(), *iter);
insert(iter.key(), iter.object());
}
}
......@@ -268,8 +268,8 @@ bool Foam::HashTable<T, Key, Hash>::set
const label hashIdx = hashKeyIndex(key);
hashedEntry* existing = 0;
hashedEntry* prev = 0;
hashedEntry* existing = nullptr;
hashedEntry* prev = nullptr;
for (hashedEntry* ep = table_[hashIdx]; ep; ep = ep->next_)
{
......@@ -281,10 +281,10 @@ bool Foam::HashTable<T, Key, Hash>::set
prev = ep;
}
// Not found, insert it at the head
if (!existing)
{
table_[hashIdx] = new hashedEntry(key, table_[hashIdx], newEntry);
// Not found, insert it at the head
table_[hashIdx] = new hashedEntry(key, newEntry, table_[hashIdx]);
nElmts_++;
if (double(nElmts_)/tableSize_ > 0.8 && tableSize_ < maxTableSize)
......@@ -316,7 +316,7 @@ bool Foam::HashTable<T, Key, Hash>::set
{
// Found - overwrite existing entry
// this corresponds to the Perl convention
hashedEntry* ep = new hashedEntry(key, existing->next_, newEntry);
hashedEntry* ep = new hashedEntry(key, newEntry, existing->next_);
// Replace existing element - within list or insert at the head
if (prev)
......@@ -342,11 +342,11 @@ bool Foam::HashTable<T, Key, Hash>::iteratorBase::erase()
if (entryPtr_)
{
// Search element before entryPtr_
hashedEntry* prev = 0;
entry_type* prev = nullptr;
for
(
hashedEntry* ep = hashTable_->table_[hashIndex_];
entry_type* ep = hashTable_->table_[hashIndex_];
ep;
ep = ep->next_
)
......@@ -371,8 +371,7 @@ bool Foam::HashTable<T, Key, Hash>::iteratorBase::erase()
hashTable_->table_[hashIndex_] = entryPtr_->next_;
delete entryPtr_;
// Assign any non-nullptr value so it doesn't look
// like end()/cend()
// Assign any non-nullptr value so it doesn't look like end()
entryPtr_ = reinterpret_cast<hashedEntry*>(this);
// Mark with special hashIndex value to signal it has been rewound.
......@@ -547,7 +546,7 @@ void Foam::HashTable<T, Key, Hash>::clear()
ep = next;
}
delete ep;
table_[hashIdx] = 0;
table_[hashIdx] = nullptr;
}
}
nElmts_ = 0;
......@@ -625,7 +624,7 @@ void Foam::HashTable<T, Key, Hash>::operator=
for (const_iterator iter = rhs.cbegin(); iter != rhs.cend(); ++iter)
{
insert(iter.key(), *iter);
insert(iter.key(), iter.object());
}
}
......@@ -667,9 +666,9 @@ bool Foam::HashTable<T, Key, Hash>::operator==
for (const_iterator iter = rhs.cbegin(); iter != rhs.cend(); ++iter)
{
const_iterator fnd = find(iter.key());
const_iterator other = find(iter.key());
if (fnd == cend() || fnd() != iter())
if (!other.found() || other.object() != iter.object())
{
return false;
}
......
......@@ -37,6 +37,7 @@ Note
SourceFiles
HashTableI.H
HashTable.C
HashTableCore.C
HashTableIO.C
\*---------------------------------------------------------------------------*/
......@@ -49,6 +50,8 @@ SourceFiles
#include "word.H"
#include "Xfer.H"
#include "className.H"
#include "nullObject.H"
#include <initializer_list>
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
......@@ -71,16 +74,26 @@ template<class T, class Key, class Hash>
Istream& operator>>(Istream& is, HashTable<T, Key, Hash>& L);
template<class T, class Key, class Hash>
Ostream& operator<<(Ostream& os, const HashTable<T, Key, Hash>& L);
Ostream& operator<<(Ostream& os, const HashTable<T, Key, Hash>& tbl);
/*---------------------------------------------------------------------------*\
Class HashTableCore Declaration
Class HashTableCore Declaration
\*---------------------------------------------------------------------------*/
//- Template-invariant bits for HashTable
struct HashTableCore
//- Template-invariant bits for HashTable.
// This also includes a global end-iterator.
//
// The end iterator of all hash-tables has a nullptr to the hash entry.
// Thus avoid separate allocation for each table and use a single one with
// a nullptr.
// The hash-table iterators always have this as its first member data,
// so we can reinterpret_cast from anything else that has a nullptr for its
// first data member. This is now also the case for the NullObject.
class HashTableCore
{
public:
//- Return a canonical (power-of-two) size
static label canonicalSize(const label size);
......@@ -94,24 +107,20 @@ struct HashTableCore
//- Define template name and debug
ClassName("HashTable");
//- A zero-sized end iterator
struct iteratorEnd
{
//- Construct null
iteratorEnd()
{}
};
//- iteratorEnd set to beyond the end of any HashTable
inline static iteratorEnd cend()
{
return iteratorEnd();
}
protected:
static_assert
(
sizeof(NullObject) >= sizeof(void*),
"NullObject is too small to reinterpret_cast as HashTable::iterator"
);
//- iteratorEnd set to beyond the end of any HashTable
inline static iteratorEnd end()
//- Reinterpret a NullObject as a hash-table iterator.
template<class Iterator>
inline static const Iterator& endIteratorRef()
{
return iteratorEnd();
return *reinterpret_cast<const Iterator*>(nullObjectPtr);
}
};
......@@ -133,14 +142,14 @@ class HashTable
//- The lookup key
Key key_;
//- Pointer to next hashedEntry in sub-list
hashedEntry* next_;
//- The data object
T obj_;
//- Pointer to next hashedEntry in sub-list
hashedEntry* next_;
//- Construct from key, next pointer and object
inline hashedEntry(const Key& key, hashedEntry* next, const T& obj);
inline hashedEntry(const Key& key, const T& obj, hashedEntry* next);
private:
......@@ -170,7 +179,7 @@ class HashTable
// No checks for zero-sized tables.
inline label hashKeyIndex(const Key& key) const;
//- Assign a new hashedEntry to a possibly already existing key.
//- Assign a new hash-entry to a possibly already existing key.
// Return true if the new entry was set.
bool set(const Key& key, const T& newEntry, const bool protect);
......@@ -187,6 +196,7 @@ protected:
const InputIter endIter
);
public:
// Forward declaration of iterators
......@@ -195,19 +205,9 @@ public:
class iterator;
class const_iterator;
//- Declare friendship with the HashPtrTable class
template<class T2, class Key2, class Hash2>
friend class HashPtrTable;
//- Declare friendship with the iteratorBase
friend class iteratorBase;
//- Declare friendship with the iterator
friend class iterator;
//- Declare friendship with the const_iterator
friend class const_iterator;
// Constructors
......@@ -351,10 +351,12 @@ public:
bool operator!=(const HashTable<T, Key, Hash>& rhs) const;
// STL type definitions
//- Type of values the HashTable contains.
//- Type of keys that the HashTable uses.
typedef Key key_type;
//- Type of values that the HashTable contains.
typedef T value_type;
//- Type that can be used for storing into HashTable::value_type
......@@ -370,6 +372,27 @@ public:
typedef label size_type;
// Iterator access
//- Iterator set to the beginning of the HashTable
inline iterator begin();
//- const_iterator set to the beginning of the HashTable
inline const_iterator begin() const;
//- const_iterator set to the beginning of the HashTable
inline const_iterator cbegin() const;
//- iterator to signal the end for any HashTable
inline const iterator& end();
//- const_iterator to signal the end for any HashTable
inline const const_iterator& end() const;
//- const_iterator to signal the end for any HashTable
inline const const_iterator& cend() const;
// Iterators and helpers
//- The iterator base for HashTable
......@@ -379,133 +402,125 @@ public:
// us to reinterpret_cast between them (if desired)
class iteratorBase
{
// Private Data
using entry_type = hashedEntry;
//- Pointer to the HashTable for which this is an iterator
// This allows use of the default bitwise copy/assignment
HashTable<T, Key, Hash>* hashTable_;
//- Current element
hashedEntry* entryPtr_;
public:
// Public typedefs
using table_type = HashTable<T, Key, Hash>;
using key_type = HashTable<T, Key, Hash>::key_type;
//- Current hash index
label hashIndex_;
private:
// Private Data
protected:
//- Currently selected entry.
// MUST be the first member for easy comparison between iterators
// and for reinterpret_cast from nullObject
entry_type* entryPtr_;
// Constructors
//- Pointer to the hash-table for which this is an iterator
// This allows use of the default bitwise copy/assignment
table_type* hashTable_;
//- Construct null - equivalent to an 'end' position
inline iteratorBase();
//- Current hash index within the hash-table data.
// A signed value, since erase() uses a negative value to signal
// the erasure state.
label hashIndex_;
//- Construct from hash table, moving to its 'begin' position
inline explicit iteratorBase
(
const HashTable<T, Key, Hash>* hashTbl
);
//- Construct from hash table, element and hash index
inline iteratorBase
(
const HashTable<T, Key, Hash>* hashTbl,
const hashedEntry* elmt,
const label hashIndex
);
protected:
// Protected Member Functions
// Protected Member Functions
//- Increment to the next position
inline void increment();
//- Increment to the next position
inline void increment();
//- Erase the entry at the current position
bool erase();
//- Erase the HashTable element at the current position
bool erase();
//- The referenced object/value element
inline T& element() const;
//- Return non-const access to referenced object
inline T& object();
public:
// Member operators
// Constructors
// Access
//- Construct null (end iterator)
inline iteratorBase();
//- True if iterator points to a hashedEntry.
// This can be used instead of a comparison to end()
inline bool found() const;
//- Construct from begin of hash-table
inline explicit iteratorBase(const table_type* hashTbl);
//- Return the Key corresponding to the iterator
inline const Key& key() const;
//- Construct from hash table, element and hash index
inline iteratorBase
(
const table_type* hashTbl,
const entry_type* elmt,
const label hashIndex
);
//- Return const access to referenced object
inline const T& cobject() const;
// Member functions/operators
//- Compare hashedEntry element pointers
inline bool operator==(const iteratorBase& iter) const;
inline bool operator!=(const iteratorBase& iter) const;
//- True if iterator points to an entry
// This can be used directly instead of comparing to end()
inline bool found() const;
//- Compare hashedEntry to iteratorEnd pointers
inline bool operator==(const iteratorEnd& unused) const;
inline bool operator!=(const iteratorEnd& unused) const;
//- Return the Key corresponding to the iterator
inline const Key& key() const;
//- Compare hash-entry element pointers
inline bool operator==(const iteratorBase& iter) const;
inline bool operator!=(const iteratorBase& iter) const;
};
// STL iterator
//- An STL-conforming iterator
class iterator
:
public iteratorBase
{
friend class HashTable;
// Private Member Functions
//- Construct from hash table, moving to its