[libcxx] r276238 - Implement std::string_view as described in http://wg21.link/P0254R1. Reviewed as https://reviews.llvm.org/D21459

[Eric Fiselier via cfe-commits]

I would also want to do serious performance analysis on this patch. Does
removing the string_view overloads cause less optimal overloads to be
chosen? Perhaps allocating ones?
That would be really unfortunate, and I'm not sure that's in the best
interest of our users at large.

/Eric

On Thu, Jun 15, 2017 at 10:51 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> On Jun 15, 2017, at 19:42, Eric Fiselier <eric at efcs.ca> wrote:
>
>
>
> On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
>
>> I just started working on a patch to add #if guards, and the first
>> interesting thing I found was the basic_string constructor:
>>
>> template <class _CharT, class _Traits, class _Allocator>
>> template <class _Tp>
>> basic_string<_CharT, _Traits, _Allocator>::basic_string(
>>              const _Tp& __t, size_type __pos, size_type __n, const
>> allocator_type& __a,
>>  typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits,
>> _Tp>::value, void>::type *)
>>     : __r_(__second_tag(), __a)
>> {
>> __self_view __sv = __self_view(__t).substr(__pos, __n);
>>     __init(__sv.data(), __sv.size());
>> #if _LIBCPP_DEBUG_LEVEL >= 2
>>     __get_db()->__insert_c(this);
>> #endif
>> }
>>
>>
>>
> That constructor was added in C++17, so removing it along with string_view
> should be OK.
> Assuming we don't use it to implement older constructors using a single
> template.
>
>
>
>> I suppose the decision was made so that std::string could take advantage
>> of it.
>>
>> Is it a conforming extension?
>>
>
> No, because it can change the meaning of otherwise well defined code, as
> you pointed out initially.
>
>
> Let me know if this patch is along the right lines.  If so, I'll finish it
> up and put it on phab.
>
> experimental/filesystem/path.cpp doesn't compile, since
> experimental/filesystem uses things like operator+=(string, string_view)
> extensively.  But I'd like an early opinion on the approach before I dig in.
>
> In string, the only function that needed to be rewritten was
> string::compare(size, size, string, size, size).  I'm nervous that
> filesystem will be a bigger job.
>
>
>
>
>
>>
>> On Jun 15, 2017, at 18:35, Eric Fiselier <eric at efcs.ca> wrote:
>>
>> It *shouldn't* include <string_view>, that's a given.
>>
>> IIRC, and Marshall would know better, I believe it was untenable to
>> maintain a version of <string> that didn't depend on <string_view> after
>> making
>> the changes required for C++17.
>>
>> However inspecting <string> now it does seem possible that the
>> entanglement
>> is avoidable.Though it's also likely I'm just not seeing the whole
>> picture.
>>
>> /Eric
>>
>> On Thu, Jun 15, 2017 at 6:42 PM, Duncan P. N. Exon Smith <
>> dexonsmith at apple.com>wrote:
>>
>>>
>>> > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits <
>>> cfe-commits at lists.llvm.org> wrote:
>>> >
>>> > Modified: libcxx/trunk/include/string
>>> > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/
>>> include/string?rev=276238&r1=276237&r2=276238&view=diff
>>> > ============================================================
>>> ==================
>>> >
>>> > @@ -435,6 +461,7 @@ basic_string<char32_t> operator "" s( co
>>> > */
>>> >
>>> > #include <__config>
>>> > +#include <string_view>
>>>
>>> This breaks the following, valid, C++14 code:
>>>
>>>     #include <string>
>>>     #include <experimental/string_view>
>>>     using namespace std;
>>>     using std::experimental::string_view;
>>>     void f() { string_view sv; }
>>>
>>> Should <string> #include <string_view> even when we're not in C++17
>>> mode?  Why?
>>>
>>> > #include <iosfwd>
>>> > #include <cstring>
>>> > #include <cstdio>  // For EOF.
>>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170615/5b89cff1/attachment.html>