Re: Implement N3762 string_view: a non-owning reference to a string.

[Ed Smith-Rowland <3dw4rd at verizon dot net>]

On 11/15/2013 06:18 AM, Jonathan Wakely wrote:
> On 15 November 2013 04:42, Ed Smith-Rowland wrote:
> > As the title says.  It's a pretty simple class" a non-mutating read-only
> > view into a const CharT* or a basic_string.
> Nice, thanks.
>
> +/** @file experimantal/string_view
> + *  This is a Standard C++ Library header.
> + */
>
> s/experimantal/experimental/
> The same doxygen header appears in string_view.tcc but shouldn't be
> there (it should either have no doxygen @file block or should have a
> "This is an internal header file, ..." block.
>
> Can you use the same approach as std::array::at to make
> basic_string_view::at constexpr?
>
> basic_string_view::substr would need a constexpr std::min to use the
> same technique.
>
> _S_compare should be possible to make constexpr, I can look into that
> once it's committed.
>
> +  namespace __detail
> +  {
> +    //  Identiry transform to make ADL work with just one argument.
> +    //  See n3766.html.
> +    template<class _Tp=void>
> +      struct __identity
> +      { typedef _Tp type; };
> +
> +    template<>
> +      struct __identity<void>;
> +
> +    template<class _Tp>
> +      using __idt = typename __identity<_Tp>::type;
> +  }
>
> s/Identiry/Identity/ and please use typename _Tp instead of class _Tp
> in the template parameter list.
>
> +  template<typename _CharT, typename _Traits>
> +    bool
> +    operator==(basic_string_view<_CharT, _Traits> __x,
> +               basic_string_view<_CharT, _Traits> __y) noexcept
> +    { return __x.compare(__y) == 0; }
> +  template<typename _CharT, typename _Traits>
> +    bool
> +    operator==(basic_string_view<_CharT, _Traits> __x,
> +               __detail::__idt<basic_string_view<_CharT, _Traits>>
> __y) noexcept
> +    { return __x.compare(__y) == 0; }
> +  template<typename _CharT, typename _Traits>
> +    bool
> +    operator==(__detail::__idt<basic_string_view<_CharT, _Traits>> __x,
> +               basic_string_view<_CharT, _Traits> __y) noexcept
> +    { return __x.compare(__y) == 0; }
>
> Please add a blank line between these function templates.
>
> I think the std::hash declaration and specializations should be inside
> a _GLIBCXX_BEGIN_NAMESPACE_VERSION / _GLIBCXX_END_NAMESPACE_VERSION
> pair, because when using versioned namespaces they will be in
> std::__v7 not std.
Here is a new patch that I believe answers all questions.

Fixed spelling.

For at, string_view isn't templatized on size so I can't have different 
reps for the 0-sized case.  I do return with the save thing as array 
does for out of bounds.

I took a shot at substr using the same mechanism.

I fixed added spaces and there were some >80 lines i fixed.

I did class -> typename.

_S_compare I did the same trick as with at and substr to get constexpr.  
It's sloppy because since we don't have C++14 constexpr yet I can't 
store a variable for __diff.  The repeated difference calculation will 
either be done at compile time or hopefully optimized out.

I fixed a test case that I missed that was failing.

Open issues:
I added operator""sv on my own initiative.  This has been mentioned as a 
possibility in all drafts that I've seen but it's still an addition.
The papers have =default copy ctor and copy assign.  Why not the move 
ctor and move assign?
This isn't officially in a TS yet.

OK?

Ed

Attachment: CL_string_view
Description: Text document

Attachment: patch_string_view_2
Description: Text document