[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_view:216
@@ +215,3 @@
+	basic_string_view(const _CharT* __s)
+		: __data(__s), __size(_Traits::length(__s)) {}
+
----------------
mclow.lists wrote:
> kimgr wrote:
> > mclow.lists wrote:
> > > kimgr wrote:
> > > > I'm working from the paper at https://isocpp.org/files/papers/N3762.html, and I find it a little sketchy on the policy for nullptrs.
> > > > 
> > > > Since the ctor above accepts nullptr as long as the length is zero, would it make sense to do that here too? That is, only call _Traits::length for non-nullptr __s args?
> > > Reading from N4600: Requires: `[str, str + traits::length(str))` is a valid range.
> > > 
> > > So, no - passing `nullptr` here is undefined.
> > > 
> > OK, that feels more principled to me, anyway.
> > 
> > But the `(const char*, size_t)` constructor has the same requirement and it *does* accept `nullptr`, provided the length is zero. I saw you had to get rid of the assertion, but I feel like the constructor should probably not silently accept `nullptr` for zero-sized strings. Or do you know if that's intentional? Many StringRef/StringPiece implementations seem to have the same behavior.
> It is absolutely intentional. `[nullptr, nullptr+0)` is a perfectly fine half-open range. It contains no characters.  
> 
> However, the ctor that takes just a pointer has to calculate the length .. by dereferencing the pointer.
> 
> I had to get rid of the assertion because one of the bots (gcc 4.9) has a bug about constexpr ctors in c++11 mode.  Even though the assertion was `#ifdef`ed on C++ > 11, the compiler complained about it.  I'll be putting the assertion back as soon as I can figure out how to keep gcc from complaining.
This was discussed (at length) in LEWG during the design of `string_view`.


https://reviews.llvm.org/D21459