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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone 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
+
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113161