Quuxplusone updated this revision to Diff 109678.
Quuxplusone added a comment.
I've updated https://reviews.llvm.org/D35863 to be actually correct AFAICT from
my local testing; but I'm not sure what's the most appropriate way to get
"fancy allocator" tests into libcxx's test suite. The way I did it locally is
here:
https://github.com/Quuxplusone/libcxx/pull/1/files
Basically, I conditionally replace "test_allocator.h" with
"test_fancy_allocator.h", and then re-run all the existing tests.
"test_fancy_allocator" uses fancy pointers that carry with them a "payload" of
the allocated size `n`, and then in `a.deallocate(p,n)` we assert that
`p.payload()==n`. A bunch of list tests fail this assertion before this patch,
and none fail after this patch.
@ericwf re our Slack conversation yesterday afternoon: Am I correct that
changing the fancy pointer to a raw pointer in `iterator::operator->()` like
this would be an ABI-breaking change and therefore a non-starter?
https://github.com/Quuxplusone/libcxx/commit/c80f4ffad2ad04e8749b162629255b191c896b4f
Even if *that* change were friendly, would I be correct that changing the fancy
pointer to a raw pointer in the data member `iterator::__ptr_` would be an
ABI-breaking change and therefore a non-starter? (That would also require more
drastic surgery in `list::erase(p)`, because we could no longer trust
`p.__ptr_` to be deallocatable; we'd have to compute `q =
p.__ptr_->__prev_->__next_` and deallocate *that*. I have not attempted to
write that patch, because I assume it'd be ABI-breaking anyway.)
https://reviews.llvm.org/D35863
Files:
include/list
Index: include/list
===================================================================
--- include/list
+++ include/list
@@ -261,11 +261,6 @@
typedef __list_node_base<_Tp, _VoidPtr> __base;
typedef typename __base::__link_pointer __link_pointer;
-
- _LIBCPP_INLINE_VISIBILITY
- __link_pointer __as_link() {
- return static_cast<__link_pointer>(__base::__self());
- }
};
template <class _Tp, class _Alloc = allocator<_Tp> > class _LIBCPP_TEMPLATE_VIS list;
@@ -728,7 +723,7 @@
__sz() = 0;
while (__f != __l)
{
- __node_pointer __np = __f->__as_node();
+ __node_pointer __np = static_cast<__node_pointer>(__f);
__f = __f->__next_;
__node_alloc_traits::destroy(__na, _VSTD::addressof(__np->__value_));
__node_alloc_traits::deallocate(__na, __np, 1);
@@ -1392,12 +1387,12 @@
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__hold->__prev_ = 0;
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
- __link_nodes(__p.__ptr_, __hold->__as_link(), __hold->__as_link());
+ __link_nodes(__p.__ptr_, static_cast<__link_pointer>(__hold.get()), static_cast<__link_pointer>(__hold.get()));
++base::__sz();
#if _LIBCPP_DEBUG_LEVEL >= 2
- return iterator(__hold.release()->__as_link(), this);
+ return iterator(static_cast<__link_pointer>(__hold.release()), this);
#else
- return iterator(__hold.release()->__as_link());
+ return iterator(static_cast<__link_pointer>(__hold.release()));
#endif
}
@@ -1423,9 +1418,9 @@
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
++__ds;
#if _LIBCPP_DEBUG_LEVEL >= 2
- __r = iterator(__hold->__as_link(), this);
+ __r = iterator(static_cast<__link_pointer>(__hold.get()), this);
#else
- __r = iterator(__hold->__as_link());
+ __r = iterator(static_cast<__link_pointer>(__hold.get()));
#endif
__hold.release();
iterator __e = __r;
@@ -1437,7 +1432,7 @@
{
__hold.reset(__node_alloc_traits::allocate(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
- __e.__ptr_->__next_ = __hold->__as_link();
+ __e.__ptr_->__next_ = static_cast<__link_pointer>(__hold.get());
__hold->__prev_ = __e.__ptr_;
__hold.release();
}
@@ -1449,7 +1444,7 @@
{
__node_alloc_traits::destroy(__na, _VSTD::addressof(*__e));
__link_pointer __prev = __e.__ptr_->__prev_;
- __node_alloc_traits::deallocate(__na, __e.__ptr_->__as_node(), 1);
+ __node_alloc_traits::deallocate(__na, static_cast<__node_pointer>(__e.__ptr_), 1);
if (__prev == 0)
break;
#if _LIBCPP_DEBUG_LEVEL >= 2
@@ -1491,9 +1486,9 @@
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), *__f);
++__ds;
#if _LIBCPP_DEBUG_LEVEL >= 2
- __r = iterator(__hold.get()->__as_link(), this);
+ __r = iterator(static_cast<__link_pointer>(__hold.get()), this);
#else
- __r = iterator(__hold.get()->__as_link());
+ __r = iterator(static_cast<__link_pointer>(__hold.get()));
#endif
__hold.release();
iterator __e = __r;
@@ -1505,7 +1500,7 @@
{
__hold.reset(__node_alloc_traits::allocate(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), *__f);
- __e.__ptr_->__next_ = __hold.get()->__as_link();
+ __e.__ptr_->__next_ = static_cast<__link_pointer>(__hold.get());
__hold->__prev_ = __e.__ptr_;
__hold.release();
}
@@ -1517,7 +1512,7 @@
{
__node_alloc_traits::destroy(__na, _VSTD::addressof(*__e));
__link_pointer __prev = __e.__ptr_->__prev_;
- __node_alloc_traits::deallocate(__na, __e.__ptr_->__as_node(), 1);
+ __node_alloc_traits::deallocate(__na, static_cast<__node_pointer>(__e.__ptr_), 1);
if (__prev == 0)
break;
#if _LIBCPP_DEBUG_LEVEL >= 2
@@ -1543,7 +1538,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
- __link_pointer __nl = __hold->__as_link();
+ __link_pointer __nl = static_cast<__link_pointer>(__hold.get());
__link_nodes_at_front(__nl, __nl);
++base::__sz();
__hold.release();
@@ -1557,7 +1552,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
- __link_nodes_at_back(__hold.get()->__as_link(), __hold.get()->__as_link());
+ __link_nodes_at_back(static_cast<__link_pointer>(__hold.get()), static_cast<__link_pointer>(__hold.get()));
++base::__sz();
__hold.release();
}
@@ -1572,7 +1567,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::move(__x));
- __link_nodes_at_front(__hold.get()->__as_link(), __hold.get()->__as_link());
+ __link_nodes_at_front(static_cast<__link_pointer>(__hold.get()), static_cast<__link_pointer>(__hold.get()));
++base::__sz();
__hold.release();
}
@@ -1585,7 +1580,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::move(__x));
- __link_nodes_at_back(__hold.get()->__as_link(), __hold.get()->__as_link());
+ __link_nodes_at_back(static_cast<__link_pointer>(__hold.get()), static_cast<__link_pointer>(__hold.get()));
++base::__sz();
__hold.release();
}
@@ -1603,7 +1598,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::forward<_Args>(__args)...);
- __link_nodes_at_front(__hold.get()->__as_link(), __hold.get()->__as_link());
+ __link_nodes_at_front(static_cast<__link_pointer>(__hold.get()), static_cast<__link_pointer>(__hold.get()));
++base::__sz();
#if _LIBCPP_STD_VER > 14
return __hold.release()->__value_;
@@ -1625,7 +1620,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::forward<_Args>(__args)...);
- __link_pointer __nl = __hold->__as_link();
+ __link_pointer __nl = static_cast<__link_pointer>(__hold.get());
__link_nodes_at_back(__nl, __nl);
++base::__sz();
#if _LIBCPP_STD_VER > 14
@@ -1650,7 +1645,7 @@
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__hold->__prev_ = 0;
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::forward<_Args>(__args)...);
- __link_pointer __nl = __hold.get()->__as_link();
+ __link_pointer __nl = static_cast<__link_pointer>(__hold.get());
__link_nodes(__p.__ptr_, __nl, __nl);
++base::__sz();
__hold.release();
@@ -1675,7 +1670,7 @@
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__hold->__prev_ = 0;
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::move(__x));
- __link_pointer __nl = __hold->__as_link();
+ __link_pointer __nl = static_cast<__link_pointer>(__hold.get());
__link_nodes(__p.__ptr_, __nl, __nl);
++base::__sz();
__hold.release();
@@ -1712,7 +1707,7 @@
}
__get_db()->unlock();
#endif
- __node_pointer __np = __n->__as_node();
+ __node_pointer __np = static_cast<__node_pointer>(__n);
__node_alloc_traits::destroy(__na, _VSTD::addressof(__np->__value_));
__node_alloc_traits::deallocate(__na, __np, 1);
}
@@ -1741,7 +1736,7 @@
}
__get_db()->unlock();
#endif
- __node_pointer __np = __n->__as_node();
+ __node_pointer __np = static_cast<__node_pointer>(__n);
__node_alloc_traits::destroy(__na, _VSTD::addressof(__np->__value_));
__node_alloc_traits::deallocate(__na, __np, 1);
}
@@ -1777,7 +1772,7 @@
}
__get_db()->unlock();
#endif
- __node_pointer __np = __n->__as_node();
+ __node_pointer __np = static_cast<__node_pointer>(__n);
__node_alloc_traits::destroy(__na, _VSTD::addressof(__np->__value_));
__node_alloc_traits::deallocate(__na, __np, 1);
#if _LIBCPP_DEBUG_LEVEL >= 2
@@ -1823,7 +1818,7 @@
}
__get_db()->unlock();
#endif
- __node_pointer __np = __n->__as_node();
+ __node_pointer __np = static_cast<__node_pointer>(__n);
__node_alloc_traits::destroy(__na, _VSTD::addressof(__np->__value_));
__node_alloc_traits::deallocate(__na, __np, 1);
}
@@ -1852,9 +1847,9 @@
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_));
++__ds;
#if _LIBCPP_DEBUG_LEVEL >= 2
- iterator __r = iterator(__hold.release()->__as_link(), this);
+ iterator __r = iterator(static_cast<__link_pointer>(__hold.release()), this);
#else
- iterator __r = iterator(__hold.release()->__as_link());
+ iterator __r = iterator(static_cast<__link_pointer>(__hold.release()));
#endif
iterator __e = __r;
#ifndef _LIBCPP_NO_EXCEPTIONS
@@ -1865,7 +1860,7 @@
{
__hold.reset(__node_alloc_traits::allocate(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_));
- __e.__ptr_->__next_ = __hold.get()->__as_link();
+ __e.__ptr_->__next_ = static_cast<__link_pointer>(__hold.get());
__hold->__prev_ = __e.__ptr_;
__hold.release();
}
@@ -1877,7 +1872,7 @@
{
__node_alloc_traits::destroy(__na, _VSTD::addressof(*__e));
__link_pointer __prev = __e.__ptr_->__prev_;
- __node_alloc_traits::deallocate(__na, __e.__ptr_->__as_node(), 1);
+ __node_alloc_traits::deallocate(__na, static_cast<__node_pointer>(__e.__ptr_), 1);
if (__prev == 0)
break;
#if _LIBCPP_DEBUG_LEVEL >= 2
@@ -1910,7 +1905,7 @@
__hold->__prev_ = 0;
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
++__ds;
- __link_pointer __nl = __hold.release()->__as_link();
+ __link_pointer __nl = static_cast<__link_pointer>(__hold.release());
#if _LIBCPP_DEBUG_LEVEL >= 2
iterator __r = iterator(__nl, this);
#else
@@ -1925,7 +1920,7 @@
{
__hold.reset(__node_alloc_traits::allocate(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
- __e.__ptr_->__next_ = __hold.get()->__as_link();
+ __e.__ptr_->__next_ = static_cast<__link_pointer>(__hold.get());
__hold->__prev_ = __e.__ptr_;
__hold.release();
}
@@ -1937,7 +1932,7 @@
{
__node_alloc_traits::destroy(__na, _VSTD::addressof(*__e));
__link_pointer __prev = __e.__ptr_->__prev_;
- __node_alloc_traits::deallocate(__na, __e.__ptr_->__as_node(), 1);
+ __node_alloc_traits::deallocate(__na, static_cast<__node_pointer>(__e.__ptr_), 1);
if (__prev == 0)
break;
#if _LIBCPP_DEBUG_LEVEL >= 2
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
