[PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

[Marshall Clow via cfe-commits]

mclow.lists added inline comments.

================
Comment at: include/string:3216
@@ -3668,3 +3215,3 @@
 {
-    return _VSTD::__str_find_last_not_of<value_type, size_type, traits_type, npos>
+    return __str_find_last_not_of<value_type, size_type, traits_type, npos>
         (data(), size(), __str.data(), __pos, __str.size());
----------------
EricWF wrote:
> Why is this call no longer qualified? It has UDT's that can force ADL.
Because it has a reserved name. Users cannot provide a function with this name in their programs.

================
Comment at: include/string_view:209
@@ +208,3 @@
+	{
+//             _LIBCPP_ASSERT(__len == 0 || __s != nullptr, "string_view::string_view(_CharT *, size_t): received nullptr");
+	}
----------------
EricWF wrote:
> I think we have two choices for this diagnostic:
> 
> 1. Remove it, since it will never work with C++11 constexpr.
> 2. Only expose the assertion in C++14 or newer.
> 3. Only may this constructor constexpr in C++14, and keep the assertion.
> 
> (3) seems like a Good option to me since offering string_view in C++11 is an extension anyway.
Either #2 or #3 is fine with me.


================
Comment at: include/string_view:331
@@ +330,3 @@
+	{
+// 		if (__pos > size())
+// 			throw out_of_range("string_view::substr");
----------------
EricWF wrote:
> These lines should be removed.
Yeah. Frackin' C++11 constexpr.



https://reviews.llvm.org/D21459