The code, as it stands, looks fine.
I check the code from libc++ (relevant parts) and I believe that it just confuses the static analyzer.
In more details:
template <class _Tp, class _Alloc>
void list<_Tp, _Alloc>::pop_front()
{
_LIBCPP_ASSERT(!empty(), "list::pop_front() called with empty list");
__node_allocator& __na = base::__node_alloc();
__node_pointer __n = base::__end_.__next_;
base::__unlink_nodes(__n, __n);
--base::__sz();
__node_alloc_traits::destroy(__na, _VSTD::addressof(__n->__value_));
__node_alloc_traits::deallocate(__na, __n, 1);
}
list
is implemented as a circular list, based at __end_
(which is the end-pointer), so to get to the first element, the code goes to __end_.__next_
.
The implementation of __unlink_nodes
is:
// Unlink nodes [__f, __l]
template <class _Tp, class _Alloc>
inline void __list_imp<_Tp, _Alloc>::__unlink_nodes(__node_pointer __f,
__node_pointer __l) noexcept
{
__f->__prev_->__next_ = __l->__next_;
__l->__next_->__prev_ = __f->__prev_;
}
We can easily understand it with some simple ASCII art:
Z A B C
+---------+ +---------+ +---------+ +---------+
--| __prev_ |<--| __prev_ |<--| __prev_ |<--| __prev_ |<-
->| __next_ |-->| __next_ |-->| __next_ |-->| __next_ |--
+---------+ +---------+ +---------+ +---------+
To remove the range A
-B
from this list:
Z.__next_
has to point to C
C.__prev_
has to point to Z
Thus the call __unlink_nodes(A, B)
will:
- take
A.__prev_.__next_
(ie, Z.__next_
) and make it point to B.__next_
(ie, C
)
- take
B.__next_.__prev_
(ie, C.__prev_
) and make it point to A.__prev_
(ie, Z
)
This is simple, and works even when invoked with a single element-range (the case here).
Now, however, note that if the list
were to be empty, this would not work at all! The default constructor of __list_node_base
is:
__list_node_base()
: __prev_(static_cast<pointer>(pointer_traits<__base_pointer>::pointer_to(*this))),
__next_(static_cast<pointer>(pointer_traits<__base_pointer>::pointer_to(*this)))
{}
That is, it refers to itself. In this case, __unlink_nodes
is invoked with &__end_
(twice), and would not change it __end_.__prev_.__next_ = __end_.__next_
is idempotent (because __end_.prev
is __end_
itself).
It may be that:
- the analyzer takes into account the case of an empty list (
_LIBCPP_ASSERT
being compiled out)
- and concludes that in this case, the
__end_.__next_
(used by begin()
) is left dangling by the deallocate()
call in pop_front()
Or maybe it's something else in the pointer dance... hopefully the Clang team will be able to patch things up.