Conversation
…l traversal Doubly-linked list with fixed capacity (power-of-2), immediate node recycling via free list, and lazy allocation. Includes 32 unit tests covering all operations.
|
|
||
| The input and output structs of contract user procedures and functions may only use integer and boolean types (such as `uint64`, `sint8`, `bit`) as well as `id`, `Array`, and `BitArray`, and struct types containing only allowed types. | ||
| Complex types that may have an inconsistent internal state, such as `Collection`, are forbidden in the public interface of a contract. | ||
| Complex types that may have an inconsistent internal state, such as `Collection` and `LinkedList`, are forbidden in the public interface of a contract. |
There was a problem hiding this comment.
are you going to add this to the contract-verify check?
There was a problem hiding this comment.
Should already be covered by not being in the allowlist, or do I miss something?
| { | ||
| if (elementIndex < 0 || elementIndex >= (sint64)L) | ||
| return false; | ||
| if (!(_occupiedFlags[elementIndex >> 6] & (1ULL << (elementIndex & 63)))) |
There was a problem hiding this comment.
this check is repeated very often. I think it would be worth it to add a helper function like isInRangeAndOccupied
| void _freeNode(sint64 nodeIndex); | ||
|
|
||
| public: | ||
| // Return maximum number of elements that may be stored. |
There was a problem hiding this comment.
maybe I overlooked it somewhere but I suggest to add a constructor that calls reset() to make sure that everything is initialized properly
There was a problem hiding this comment.
Unfortunately, the constructor isn't called when this List is used in a contract state. The state is just initialized with all-0. So either the contract dev MUST call reset() or we need to add a mechanism that checks _population == 0 and initializes _headIndex, _tailIndex, and _freeHeadIndex as NULL_INDEX in this case. This init check could be done at the beginning of each non-const method. The const methods headIndex() and tailIndex() could check for _population == 0 and return NULL_INDEX in this case as well.
philippwerner
left a comment
philippwerner
left a comment
There was a problem hiding this comment.
Overall, this looks good to me. The only issue I see is the initialization (see my response to Franziska's comment above) that we should either address in the code (auto-init if population is 0) or in the documentation + maybe contract checking tool (make sure that reset() is called in INITIALIZE).
| // Return element value at elementIndex. | ||
| inline const T& element(sint64 elementIndex) const; | ||
|
|
||
| // Return true if the slot at elementIndex is empty (not occupied by an element). |
There was a problem hiding this comment.
I would add that out-of-range elements are considered empty here.
| void _freeNode(sint64 nodeIndex); | ||
|
|
||
| public: | ||
| // Return maximum number of elements that may be stored. |
There was a problem hiding this comment.
Unfortunately, the constructor isn't called when this List is used in a contract state. The state is just initialized with all-0. So either the contract dev MUST call reset() or we need to add a mechanism that checks _population == 0 and initializes _headIndex, _tailIndex, and _freeHeadIndex as NULL_INDEX in this case. This init check could be done at the beginning of each non-const method. The const methods headIndex() and tailIndex() could check for _population == 0 and return NULL_INDEX in this case as well.
3 participants
Adds LinkedList<T, L> to the QPI — a doubly-linked list with fixed capacity L (must be power of 2).