Re: [PATCH] D10677: Allow deque to handle incomplete types
eugenis set the repository for this revision to rL LLVM.
eugenis updated this revision to Diff 37310.
eugenis added a comment.
Using new ABI version macros to enable this feature in unstable or future ABI
only.
PTAL.
Repository:
rL LLVM
http://reviews.llvm.org/D10677
Files:
include/__config
include/deque
test/std/containers/sequences/deque/deque.cons/incomplete.pass.cpp
Index: test/std/containers/sequences/deque/deque.cons/incomplete.pass.cpp
===
--- /dev/null
+++ test/std/containers/sequences/deque/deque.cons/incomplete.pass.cpp
@@ -0,0 +1,31 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// deque()
+// deque::iterator()
+
+#define _LIBCPP_ABI_INCOMPLETE_TYPES_IN_DEQUE
+#include
+#include
+
+struct A {
+ std::deque d;
+ std::deque::iterator it;
+ std::deque::reverse_iterator it2;
+};
+
+int main()
+{
+ A a;
+ assert(a.d.size() == 0);
+ a.it = a.d.begin();
+ a.it2 = a.d.rend();
+}
Index: include/deque
===
--- include/deque
+++ include/deque
@@ -261,8 +261,19 @@
__deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1> __l,
__deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2> __r);
+template
+struct __deque_block_size {
+ static const _DiffType value = sizeof(_ValueType) < 256 ? 4096 / sizeof(_ValueType) : 16;
+};
+
template
+ class _DiffType, _DiffType _BS =
+#ifdef _LIBCPP_ABI_INCOMPLETE_TYPES_IN_DEQUE
+ 0
+#else
+ __deque_block_size<_ValueType, _DiffType>::value
+#endif
+ >
class _LIBCPP_TYPE_VIS_ONLY __deque_iterator
{
typedef _MapPointer __map_iterator;
@@ -273,7 +284,7 @@
__map_iterator __m_iter_;
pointer__ptr_;
-static const difference_type __block_size = _BlockSize;
+static const difference_type __block_size;
public:
typedef _ValueType value_type;
typedef random_access_iterator_tag iterator_category;
@@ -287,7 +298,7 @@
template
_LIBCPP_INLINE_VISIBILITY
-__deque_iterator(const __deque_iterator& __it,
+__deque_iterator(const __deque_iterator& __it,
typename enable_if::value>::type* = 0) _NOEXCEPT
: __m_iter_(__it.__m_iter_), __ptr_(__it.__ptr_) {}
@@ -520,6 +531,12 @@
__deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2> __r);
};
+template
+const _DiffType __deque_iterator<_ValueType, _Pointer, _Reference, _MapPointer,
+ _DiffType, _BlockSize>::__block_size =
+__deque_block_size<_ValueType, _DiffType>::value;
+
// copy
template ::difference_type difference_type;
typedef typename __deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2>::pointer pointer;
+const difference_type __block_size = __deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2>::__block_size;
while (__f != __l)
{
pointer __rb = __r.__ptr_;
-pointer __re = *__r.__m_iter_ + _B2;
+pointer __re = *__r.__m_iter_ + __block_size;
difference_type __bs = __re - __rb;
difference_type __n = __l - __f;
_RAIter __m = __l;
@@ -560,11 +578,12 @@
{
typedef typename __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::difference_type difference_type;
typedef typename __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::pointer pointer;
+const difference_type __block_size = __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::__block_size;
difference_type __n = __l - __f;
while (__n > 0)
{
pointer __fb = __f.__ptr_;
-pointer __fe = *__f.__m_iter_ + _B1;
+pointer __fe = *__f.__m_iter_ + __block_size;
difference_type __bs = __fe - __fb;
if (__bs > __n)
{
@@ -587,11 +606,12 @@
{
typedef typename __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::difference_type difference_type;
typedef typename __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::pointer pointer;
+const difference_type __block_size = __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::__block_size;
difference_type __n = __l - __f;
while (__n > 0)
{
pointer __fb = __f.__ptr_;
-pointer __fe = *__f.__m_iter_ + _B1;
+pointer __fe = *__f.__m_iter_ + __block_size;
difference_type __bs = __fe - __fb;
if (__bs > __n)
{
@@ -705,10 +725,11 @@
{
typedef typename __deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2>::difference_type difference_type;
typedef typename __deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2>::pointer pointer;
+const difference_type __block_size = __de
Re: [PATCH] D10677: Allow deque to handle incomplete types
EricWF added a comment. For the most part this looks good. I'm a touch concerned though about the changes to the static initialization. The initializer is moved from within the function body to outside it. Could you have somebody confirm this won't affect the existing ABI? Comment at: include/__config:32 @@ -31,2 +31,3 @@ #define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT +#define _LIBCPP_ABI_INCOMPLETE_TYPES_IN_DEQUE #endif I think we need a short explanation of ever option added here. In this case could you just link to the bug report and a short explanaition? ex. ``` // Fix deques iterator type in order to support incomplete types. See http://llvm.org/bugs/... #define _LIBCPP_ABI_INCOMPLETE_TYPES_IN_DEQUE ``` Comment at: include/deque:270 @@ -264,2 +269,3 @@ template + class _DiffType, _DiffType _BS = +#ifdef _LIBCPP_ABI_INCOMPLETE_TYPES_IN_DEQUE Please leave a note here explaining why the template parameter was kept even though its unused. Comment at: test/std/containers/sequences/deque/deque.cons/incomplete.pass.cpp:9 @@ +8,3 @@ +//===--===// + +// This test needs to go into `test/libcxx` not `test/std` because it's testing libc++ implementation details and not standard specified behavior. Repository: rL LLVM http://reviews.llvm.org/D10677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10677: Allow deque to handle incomplete types
eugenis updated this revision to Diff 37795.
eugenis marked 3 inline comments as done.
Repository:
rL LLVM
http://reviews.llvm.org/D10677
Files:
include/__config
include/deque
test/libcxx/containers/sequences/deque/incomplete.pass.cpp
Index: test/libcxx/containers/sequences/deque/incomplete.pass.cpp
===
--- /dev/null
+++ test/libcxx/containers/sequences/deque/incomplete.pass.cpp
@@ -0,0 +1,31 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// deque()
+// deque::iterator()
+
+#define _LIBCPP_ABI_INCOMPLETE_TYPES_IN_DEQUE
+#include
+#include
+
+struct A {
+ std::deque d;
+ std::deque::iterator it;
+ std::deque::reverse_iterator it2;
+};
+
+int main()
+{
+ A a;
+ assert(a.d.size() == 0);
+ a.it = a.d.begin();
+ a.it2 = a.d.rend();
+}
Index: include/deque
===
--- include/deque
+++ include/deque
@@ -261,8 +261,21 @@
__deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1> __l,
__deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2> __r);
+template
+struct __deque_block_size {
+ static const _DiffType value = sizeof(_ValueType) < 256 ? 4096 / sizeof(_ValueType) : 16;
+};
+
template
+ class _DiffType, _DiffType _BS =
+#ifdef _LIBCPP_ABI_INCOMPLETE_TYPES_IN_DEQUE
+// Keep template parameter to avoid changing all template declarations thoughout
+// this file.
+ 0
+#else
+ __deque_block_size<_ValueType, _DiffType>::value
+#endif
+ >
class _LIBCPP_TYPE_VIS_ONLY __deque_iterator
{
typedef _MapPointer __map_iterator;
@@ -273,7 +286,7 @@
__map_iterator __m_iter_;
pointer__ptr_;
-static const difference_type __block_size = _BlockSize;
+static const difference_type __block_size;
public:
typedef _ValueType value_type;
typedef random_access_iterator_tag iterator_category;
@@ -287,7 +300,7 @@
template
_LIBCPP_INLINE_VISIBILITY
-__deque_iterator(const __deque_iterator& __it,
+__deque_iterator(const __deque_iterator& __it,
typename enable_if::value>::type* = 0) _NOEXCEPT
: __m_iter_(__it.__m_iter_), __ptr_(__it.__ptr_) {}
@@ -520,6 +533,12 @@
__deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2> __r);
};
+template
+const _DiffType __deque_iterator<_ValueType, _Pointer, _Reference, _MapPointer,
+ _DiffType, _BlockSize>::__block_size =
+__deque_block_size<_ValueType, _DiffType>::value;
+
// copy
template ::difference_type difference_type;
typedef typename __deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2>::pointer pointer;
+const difference_type __block_size = __deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2>::__block_size;
while (__f != __l)
{
pointer __rb = __r.__ptr_;
-pointer __re = *__r.__m_iter_ + _B2;
+pointer __re = *__r.__m_iter_ + __block_size;
difference_type __bs = __re - __rb;
difference_type __n = __l - __f;
_RAIter __m = __l;
@@ -560,11 +580,12 @@
{
typedef typename __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::difference_type difference_type;
typedef typename __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::pointer pointer;
+const difference_type __block_size = __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::__block_size;
difference_type __n = __l - __f;
while (__n > 0)
{
pointer __fb = __f.__ptr_;
-pointer __fe = *__f.__m_iter_ + _B1;
+pointer __fe = *__f.__m_iter_ + __block_size;
difference_type __bs = __fe - __fb;
if (__bs > __n)
{
@@ -587,11 +608,12 @@
{
typedef typename __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::difference_type difference_type;
typedef typename __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::pointer pointer;
+const difference_type __block_size = __deque_iterator<_V1, _P1, _R1, _M1, _D1, _B1>::__block_size;
difference_type __n = __l - __f;
while (__n > 0)
{
pointer __fb = __f.__ptr_;
-pointer __fe = *__f.__m_iter_ + _B1;
+pointer __fe = *__f.__m_iter_ + __block_size;
difference_type __bs = __fe - __fb;
if (__bs > __n)
{
@@ -705,10 +727,11 @@
{
typedef typename __deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2>::difference_type difference_type;
typedef typename __deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2>::pointer pointer;
+const difference_type __block_size = __deque_iterator<_V2, _P2, _R2, _M2, _D2, _B2>::__block_size;
Re: [PATCH] D10677: Allow deque to handle incomplete types
eugenis added a comment. In http://reviews.llvm.org/D10677#266595, @EricWF wrote: > For the most part this looks good. I'm a touch concerned though about the > changes to the static initialization. The initializer is moved from within > the function body to outside it. Could you have somebody confirm this won't > affect the existing ABI? I'm pretty sure it only affects template evaluation order, and does not change the mangling of any name. Repository: rL LLVM http://reviews.llvm.org/D10677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10677: Allow deque to handle incomplete types
> I'm pretty sure it only affects template evaluation order, and does not change the mangling of any name. I agree it doesn't change the mangling of any name. I just want to make sure that moving the initializer won't result in incompatible code gen. I'm sure I'm insane for being concerned about this, but I just want somebody to tell me I'm actually insane. It would make me feel better :) Sorry /Eric On Mon, Oct 19, 2015 at 11:28 AM, Evgeniy Stepanov wrote: > eugenis added a comment. > > In http://reviews.llvm.org/D10677#266595, @EricWF wrote: > > > For the most part this looks good. I'm a touch concerned though about > the changes to the static initialization. The initializer is moved from > within the function body to outside it. Could you have somebody confirm > this won't affect the existing ABI? > > > I'm pretty sure it only affects template evaluation order, and does not > change the mangling of any name. > > > Repository: > rL LLVM > > http://reviews.llvm.org/D10677 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10677: Allow deque to handle incomplete types
eugenis added a comment.
What kind of confirmation are you looking for?
I've compiled the following code with 2 versions of : one as in this
review, another the same but with __block_size initializers moved back into
respective classes. Resulting object files are identical.
#include
int main() {
std::deque d;
d.push_back(1);
d.push_back(2);
for (auto x : d) (void)x;
return d.size();
}
Repository:
rL LLVM
http://reviews.llvm.org/D10677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10677: Allow deque to handle incomplete types
eugenis added a comment. Hi Eric, could you please clarify what exactly you are looking for here? Repository: rL LLVM http://reviews.llvm.org/D10677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10677: Allow deque to handle incomplete types
EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. I think I've cleared up my own confusion. LGTM. Repository: rL LLVM http://reviews.llvm.org/D10677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10677: Allow deque to handle incomplete types
eugenis closed this revision. eugenis added a comment. Thanks! Landed as r252350. Repository: rL LLVM http://reviews.llvm.org/D10677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
