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

Thu, 15 Jun 2017 21:51:56 -0700


On Jun 15, 2017, at 19:42, Eric Fiselier <e...@efcs.ca> wrote:



On Thu, Jun 15, 2017 at 8:38 PM, Duncan P. N. Exon Smith <dexonsm...@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.

Attachment: avoid-string-view-in-cxx14.patch
Description: Binary data


 

On Jun 15, 2017, at 18:35, Eric Fiselier <e...@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 <dexonsm...@apple.com>wrote:

> On Jul 20, 2016, at 22:31, Marshall Clow via cfe-commits <cfe-commits@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.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to