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

Duncan P. N. Exon Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 15 21:51:27 PDT 2017


> 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 <mailto: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 <mailto: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 <mailto:dexonsmith at apple.com>>wrote:
>> 
>> > On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits <cfe-commits at lists.llvm.org <mailto: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 <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/a139ae6c/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avoid-string-view-in-cxx14.patch
Type: application/octet-stream
Size: 22650 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170615/a139ae6c/attachment-0001.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170615/a139ae6c/attachment-0003.html>


More information about the cfe-commits mailing list