[PATCH] D10677: Allow deque to handle incomplete types

[Eric Fiselier via cfe-commits]

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