Fix Dictionary and HashSet iterators to prevent unwanted data copies

#1361
This commit is contained in:
Wojtek Figat
2023-09-10 11:25:36 +02:00
parent 0c89aa1958
commit 9291295a4d
5 changed files with 57 additions and 52 deletions

View File

@@ -1251,7 +1251,7 @@ void VisualScriptExecutor::ProcessGroupFlow(Box* boxBase, Node* node, Value& val
boxBase = node->GetBox(3); boxBase = node->GetBox(3);
if (boxBase->HasConnection()) if (boxBase->HasConnection())
eatBox(node, boxBase->FirstConnection()); eatBox(node, boxBase->FirstConnection());
Dictionary<Variant, Variant>::Iterator it(dictionary, iteratorValue.Value.AsInt); Dictionary<Variant, Variant>::Iterator it(&dictionary, iteratorValue.Value.AsInt);
++it; ++it;
iteratorValue.Value.AsInt = it.Index(); iteratorValue.Value.AsInt = it.Index();
} }
@@ -1269,12 +1269,12 @@ void VisualScriptExecutor::ProcessGroupFlow(Box* boxBase, Node* node, Value& val
// Key // Key
case 1: case 1:
if (iteratorIndex != scope->ReturnedValues.Count() && dictionaryIndex != scope->ReturnedValues.Count()) if (iteratorIndex != scope->ReturnedValues.Count() && dictionaryIndex != scope->ReturnedValues.Count())
value = Dictionary<Variant, Variant>::Iterator(*scope->ReturnedValues[dictionaryIndex].Value.AsDictionary, scope->ReturnedValues[iteratorIndex].Value.AsInt)->Key; value = Dictionary<Variant, Variant>::Iterator(scope->ReturnedValues[dictionaryIndex].Value.AsDictionary, scope->ReturnedValues[iteratorIndex].Value.AsInt)->Key;
break; break;
// Value // Value
case 2: case 2:
if (iteratorIndex != scope->ReturnedValues.Count() && dictionaryIndex != scope->ReturnedValues.Count()) if (iteratorIndex != scope->ReturnedValues.Count() && dictionaryIndex != scope->ReturnedValues.Count())
value = Dictionary<Variant, Variant>::Iterator(*scope->ReturnedValues[dictionaryIndex].Value.AsDictionary, scope->ReturnedValues[iteratorIndex].Value.AsInt)->Value; value = Dictionary<Variant, Variant>::Iterator(scope->ReturnedValues[dictionaryIndex].Value.AsDictionary, scope->ReturnedValues[iteratorIndex].Value.AsInt)->Value;
break; break;
// Break // Break
case 5: case 5:

View File

@@ -938,12 +938,12 @@ public:
FORCE_INLINE bool IsEnd() const FORCE_INLINE bool IsEnd() const
{ {
return _index == _array->Count(); return _index == _array->_count;
} }
FORCE_INLINE bool IsNotEnd() const FORCE_INLINE bool IsNotEnd() const
{ {
return _index != _array->Count(); return _index != _array->_count;
} }
FORCE_INLINE T& operator*() const FORCE_INLINE T& operator*() const
@@ -975,7 +975,7 @@ public:
Iterator& operator++() Iterator& operator++()
{ {
if (_index != _array->Count()) if (_index != _array->_count)
_index++; _index++;
return *this; return *this;
} }
@@ -983,7 +983,7 @@ public:
Iterator operator++(int) Iterator operator++(int)
{ {
Iterator temp = *this; Iterator temp = *this;
if (_index != _array->Count()) if (_index != _array->_count)
_index++; _index++;
return temp; return temp;
} }

View File

@@ -95,7 +95,6 @@ public:
struct Iterator struct Iterator
{ {
friend ChunkedArray; friend ChunkedArray;
private: private:
ChunkedArray* _collection; ChunkedArray* _collection;
int32 _chunkIndex; int32 _chunkIndex;

View File

@@ -237,22 +237,28 @@ public:
{ {
friend Dictionary; friend Dictionary;
private: private:
Dictionary& _collection; Dictionary* _collection;
int32 _index; int32 _index;
public: public:
Iterator(Dictionary& collection, const int32 index) Iterator(Dictionary* collection, const int32 index)
: _collection(collection) : _collection(collection)
, _index(index) , _index(index)
{ {
} }
Iterator(Dictionary const& collection, const int32 index) Iterator(Dictionary const* collection, const int32 index)
: _collection((Dictionary&)collection) : _collection(const_cast<Dictionary*>(collection))
, _index(index) , _index(index)
{ {
} }
Iterator()
: _collection(nullptr)
, _index(-1)
{
}
Iterator(const Iterator& i) Iterator(const Iterator& i)
: _collection(i._collection) : _collection(i._collection)
, _index(i._index) , _index(i._index)
@@ -273,27 +279,27 @@ public:
FORCE_INLINE bool IsEnd() const FORCE_INLINE bool IsEnd() const
{ {
return _index == _collection._size; return _index == _collection->_size;
} }
FORCE_INLINE bool IsNotEnd() const FORCE_INLINE bool IsNotEnd() const
{ {
return _index != _collection._size; return _index != _collection->_size;
} }
FORCE_INLINE Bucket& operator*() const FORCE_INLINE Bucket& operator*() const
{ {
return _collection._allocation.Get()[_index]; return _collection->_allocation.Get()[_index];
} }
FORCE_INLINE Bucket* operator->() const FORCE_INLINE Bucket* operator->() const
{ {
return &_collection._allocation.Get()[_index]; return &_collection->_allocation.Get()[_index];
} }
FORCE_INLINE explicit operator bool() const FORCE_INLINE explicit operator bool() const
{ {
return _index >= 0 && _index < _collection._size; return _index >= 0 && _index < _collection->_size;
} }
FORCE_INLINE bool operator!() const FORCE_INLINE bool operator!() const
@@ -320,10 +326,10 @@ public:
Iterator& operator++() Iterator& operator++()
{ {
const int32 capacity = _collection.Capacity(); const int32 capacity = _collection->_size;
if (_index != capacity) if (_index != capacity)
{ {
const Bucket* data = _collection._allocation.Get(); const Bucket* data = _collection->_allocation.Get();
do do
{ {
_index++; _index++;
@@ -343,7 +349,7 @@ public:
{ {
if (_index > 0) if (_index > 0)
{ {
const Bucket* data = _collection._allocation.Get(); const Bucket* data = _collection->_allocation.Get();
do do
{ {
_index--; _index--;
@@ -633,7 +639,7 @@ public:
/// <param name="i">Iterator with key and value.</param> /// <param name="i">Iterator with key and value.</param>
void Add(const Iterator& i) void Add(const Iterator& i)
{ {
ASSERT(&i._collection != this && i); ASSERT(i._collection != this && i);
const Bucket& bucket = *i; const Bucket& bucket = *i;
Add(bucket.Key, bucket.Value); Add(bucket.Key, bucket.Value);
} }
@@ -667,7 +673,7 @@ public:
/// <returns>True if cannot remove item from the collection because cannot find it, otherwise false.</returns> /// <returns>True if cannot remove item from the collection because cannot find it, otherwise false.</returns>
bool Remove(const Iterator& i) bool Remove(const Iterator& i)
{ {
ASSERT(&i._collection == this); ASSERT(i._collection == this);
if (i) if (i)
{ {
ASSERT(_allocation.Get()[i._index].IsOccupied()); ASSERT(_allocation.Get()[i._index].IsOccupied());
@@ -711,7 +717,7 @@ public:
return End(); return End();
FindPositionResult pos; FindPositionResult pos;
FindPosition(key, pos); FindPosition(key, pos);
return pos.ObjectIndex != -1 ? Iterator(*this, pos.ObjectIndex) : End(); return pos.ObjectIndex != -1 ? Iterator(this, pos.ObjectIndex) : End();
} }
/// <summary> /// <summary>
@@ -812,38 +818,38 @@ public:
public: public:
Iterator Begin() const Iterator Begin() const
{ {
Iterator i(*this, -1); Iterator i(this, -1);
++i; ++i;
return i; return i;
} }
Iterator End() const Iterator End() const
{ {
return Iterator(*this, _size); return Iterator(this, _size);
} }
Iterator begin() Iterator begin()
{ {
Iterator i(*this, -1); Iterator i(this, -1);
++i; ++i;
return i; return i;
} }
FORCE_INLINE Iterator end() FORCE_INLINE Iterator end()
{ {
return Iterator(*this, _size); return Iterator(this, _size);
} }
const Iterator begin() const const Iterator begin() const
{ {
Iterator i(*this, -1); Iterator i(this, -1);
++i; ++i;
return i; return i;
} }
FORCE_INLINE const Iterator end() const FORCE_INLINE const Iterator end() const
{ {
return Iterator(*this, _size); return Iterator(this, _size);
} }
protected: protected:

View File

@@ -213,17 +213,17 @@ public:
{ {
friend HashSet; friend HashSet;
private: private:
HashSet& _collection; HashSet* _collection;
int32 _index; int32 _index;
Iterator(HashSet& collection, const int32 index) Iterator(HashSet* collection, const int32 index)
: _collection(collection) : _collection(collection)
, _index(index) , _index(index)
{ {
} }
Iterator(HashSet const& collection, const int32 index) Iterator(HashSet const* collection, const int32 index)
: _collection((HashSet&)collection) : _collection(const_cast<HashSet*>(collection))
, _index(index) , _index(index)
{ {
} }
@@ -244,27 +244,27 @@ public:
public: public:
FORCE_INLINE bool IsEnd() const FORCE_INLINE bool IsEnd() const
{ {
return _index == _collection.Capacity(); return _index == _collection->_size;
} }
FORCE_INLINE bool IsNotEnd() const FORCE_INLINE bool IsNotEnd() const
{ {
return _index != _collection.Capacity(); return _index != _collection->_size;
} }
FORCE_INLINE Bucket& operator*() const FORCE_INLINE Bucket& operator*() const
{ {
return _collection._allocation.Get()[_index]; return _collection->_allocation.Get()[_index];
} }
FORCE_INLINE Bucket* operator->() const FORCE_INLINE Bucket* operator->() const
{ {
return &_collection._allocation.Get()[_index]; return &_collection->_allocation.Get()[_index];
} }
FORCE_INLINE explicit operator bool() const FORCE_INLINE explicit operator bool() const
{ {
return _index >= 0 && _index < _collection._size; return _index >= 0 && _index < _collection->_size;
} }
FORCE_INLINE bool operator !() const FORCE_INLINE bool operator !() const
@@ -274,12 +274,12 @@ public:
FORCE_INLINE bool operator==(const Iterator& v) const FORCE_INLINE bool operator==(const Iterator& v) const
{ {
return _index == v._index && &_collection == &v._collection; return _index == v._index && _collection == v._collection;
} }
FORCE_INLINE bool operator!=(const Iterator& v) const FORCE_INLINE bool operator!=(const Iterator& v) const
{ {
return _index != v._index || &_collection != &v._collection; return _index != v._index || _collection != v._collection;
} }
Iterator& operator=(const Iterator& v) Iterator& operator=(const Iterator& v)
@@ -291,10 +291,10 @@ public:
Iterator& operator++() Iterator& operator++()
{ {
const int32 capacity = _collection.Capacity(); const int32 capacity = _collection->_size;
if (_index != capacity) if (_index != capacity)
{ {
const Bucket* data = _collection._allocation.Get(); const Bucket* data = _collection->_allocation.Get();
do do
{ {
_index++; _index++;
@@ -314,7 +314,7 @@ public:
{ {
if (_index > 0) if (_index > 0)
{ {
const Bucket* data = _collection._allocation.Get(); const Bucket* data = _collection->_allocation.Get();
do do
{ {
_index--; _index--;
@@ -464,7 +464,7 @@ public:
/// <param name="i">Iterator with item to add</param> /// <param name="i">Iterator with item to add</param>
void Add(const Iterator& i) void Add(const Iterator& i)
{ {
ASSERT(&i._collection != this && i); ASSERT(i._collection != this && i);
const Bucket& bucket = *i; const Bucket& bucket = *i;
Add(bucket.Item); Add(bucket.Item);
} }
@@ -498,7 +498,7 @@ public:
/// <returns>True if cannot remove item from the collection because cannot find it, otherwise false.</returns> /// <returns>True if cannot remove item from the collection because cannot find it, otherwise false.</returns>
bool Remove(const Iterator& i) bool Remove(const Iterator& i)
{ {
ASSERT(&i._collection == this); ASSERT(i._collection == this);
if (i) if (i)
{ {
ASSERT(_allocation.Get()[i._index].IsOccupied()); ASSERT(_allocation.Get()[i._index].IsOccupied());
@@ -523,7 +523,7 @@ public:
return End(); return End();
FindPositionResult pos; FindPositionResult pos;
FindPosition(item, pos); FindPosition(item, pos);
return pos.ObjectIndex != -1 ? Iterator(*this, pos.ObjectIndex) : End(); return pos.ObjectIndex != -1 ? Iterator(this, pos.ObjectIndex) : End();
} }
/// <summary> /// <summary>
@@ -559,38 +559,38 @@ public:
public: public:
Iterator Begin() const Iterator Begin() const
{ {
Iterator i(*this, -1); Iterator i(this, -1);
++i; ++i;
return i; return i;
} }
Iterator End() const Iterator End() const
{ {
return Iterator(*this, _size); return Iterator(this, _size);
} }
Iterator begin() Iterator begin()
{ {
Iterator i(*this, -1); Iterator i(this, -1);
++i; ++i;
return i; return i;
} }
FORCE_INLINE Iterator end() FORCE_INLINE Iterator end()
{ {
return Iterator(*this, _size); return Iterator(this, _size);
} }
const Iterator begin() const const Iterator begin() const
{ {
Iterator i(*this, -1); Iterator i(this, -1);
++i; ++i;
return i; return i;
} }
FORCE_INLINE const Iterator end() const FORCE_INLINE const Iterator end() const
{ {
return Iterator(*this, _size); return Iterator(this, _size);
} }
protected: protected: