[libcxx-commits] [PATCH] D113161: [libc++] Implement P1989R2: range constructor for string_view

[Joe Loser via Phabricator via libcxx-commits]

jloser marked 2 inline comments as done.
jloser added inline comments.


================
Comment at: libcxx/test/std/strings/string.view/string.view.cons/from_range.pass.cpp:115
+
+static_assert(!std::is_constructible_v<std::string_view, WithTraitsType<std::char_traits<char16_t>>>); // wrong traits_type
+
----------------
Quuxplusone wrote:
> jloser wrote:
> > Quuxplusone wrote:
> > > Either make `WithTraitsType` a non-template, or use its templateyness to prove that `string_view` //is// constructible with the //correct// traits type:
> > > ```
> > > #include "constexpr_char_traits.h"  // We have no "really weird char traits" helper header? Darn.
> > > using CCT = constexpr_char_traits<char>;
> > > static_assert(std::is_constructible_v<std::string_view, WithTraitsType<std::char_traits<char>>>);
> > > static_assert(std::is_constructible_v<std::wstring_view, WithTraitsType<std::char_traits<wchar_t>>>);
> > > static_assert(std::is_constructible_v<std::basic_string_view<char, CCT>, WithTraitsType<CCT>>);
> > > static_assert(!std::is_constructible_v<std::string_view, WithTraitsType<CCT>>);  // wrong traits type
> > > static_assert(!std::is_constructible_v<std::wstring_view, WithTraitsType<std::char_traits<char>>>);  // wrong traits type
> > > ```
> > > Here I'm also drive-by testing that we're looking at the whole traits type, not just its character type.
> > I like that idea -- it actually caught a bug in our implementation since I was comparing the `traits_type` with `_Char` and not `_Traits`!. Still need to figure out why our implementation is rejecting 
> > 
> > ```
> > static_assert(std::is_constructible_v<std::wstring_view, WithTraitsType<std::char_traits<wchar_t>>>);
> > ```
> That'll be because if you're keeping `WithTraitsType` as a template, then you must make it respect its traits type in `begin` and `end`.
> ```
> template<class CharTraits>
> struct WithTraitsType {
>   typename CharTraits::char_type *begin() const;
>   typename CharTraits::char_type *end() const;
>   using traits_type = CharTraits;
> };
> ```
> should fix it.
Right, that's it. That skipped right past my brain this morning before coffee. :)

I left the `CharTraits` as a template.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113161/new/

https://reviews.llvm.org/D113161