implement intrusive list - #1664
Conversation
LLirikkkk
added
runtime
| } | ||
|
|
||
| auto is_linked() const noexcept -> bool { | ||
| return m_prev != this; |
There was a problem hiding this comment.
Let's add a comment explaining why it's ok to only check m_prev here
| }; | ||
|
|
||
| template<typename T, typename... Tags> | ||
| auto make_list_node(T value) noexcept -> list_node<T, Tags...> { |
There was a problem hiding this comment.
Looks like we don't need this one in public API. list_node's constructors are enough. What do you think?
Also, I think it's worth it to add an in-place constructor like:
template<typename... Args>
explicit list_node(std::in_place_t, Args&&... args)
: m_value{std::forward<Args>(args)...} {}
so we can eliminate redundant move in some cases
| details::list_node_base* m_curr{nullptr}; | ||
|
|
||
| explicit list_iterator(const details::list_node_base* node) noexcept | ||
| : m_curr{const_cast<details::list_node_base*>(node)} {} |
There was a problem hiding this comment.
Let's add a comment justifying this const_cast
| public: | ||
| using value_type = typename Node::value_type; | ||
| using size_type = size_t; | ||
| using difference_type = ptrdiff_t; |
There was a problem hiding this comment.
Let's use std::ptrdiff_t as you do in the iterator
There was a problem hiding this comment.
I think we can easily get rid of it. vk::intrusive::list is enough
There was a problem hiding this comment.
- missing copyright header
- let's reorder includes here: take a look at
zlib-functions.hfor reference - let's also add a test that covers
list_node's default constructor
| } | ||
|
|
||
| auto clear() noexcept -> void { | ||
| m_sentinel.unlink(); |
There was a problem hiding this comment.
Should we add a comment that nodes keep technically linked after this operation?
|
|
||
| struct default_tag {}; | ||
|
|
||
| template<typename T, typename... Tags> |
There was a problem hiding this comment.
What will happen if Tags have a duplicate?
3 participants
No description provided.