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 _ValueType, class _Pointer, class _Reference, class 
_MapPointer,
-          class _DiffType, _DiffType _BlockSize>
+          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 @@
+//===----------------------------------------------------------------------===//
+
+// <deque>
----------------
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

Reply via email to