ldionne added a comment.

LGTM. All comments/questions are just for my education. Other things I did: 
double-check that you changed all the functions changed by 
https://cplusplus.github.io/LWG/lwg-defects.html#2946.



================
Comment at: include/memory:5647
+       typename __void_t<typename _Alloc::value_type>::type,
+       typename 
__void_t<decltype(_VSTD::declval<_Alloc&>().allocate(size_t(0)))>::type
+     >
----------------
Sorry -- still not very fluent with how things are done in libc++, but don't we 
need to guard this based on C++11 at the very least because it's using 
`decltype`?


================
Comment at: include/string:842
+        explicit basic_string(const _Tp& __t,
+                     typename 
enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, 
void>::type* = 0);
+
----------------
Is there a reason why you use a different `enable_if` pattern here (as a 
default argument) and above (as a default template argument)?


================
Comment at: include/string:1646
+         class _Allocator = allocator<_CharT>,
+         class = typename enable_if<__is_allocator<_Allocator>::value, 
void>::type
+         >
----------------
You don't need to specify the `void` in `enable_if<__is_allocator, 
void>::type`. There's no harm in specifying it, but I'm curious to know if 
there's a reason for it?


================
Comment at: include/string:1779
 template <class _CharT, class _Traits, class _Allocator>
-inline _LIBCPP_INLINE_VISIBILITY
+template <class>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT* __s)
----------------
Wow, it's terrible that we need to write this.


https://reviews.llvm.org/D48616



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to