From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 107716 invoked by alias); 6 Apr 2018 20:37:15 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 107699 invoked by uid 89); 6 Apr 2018 20:37:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD,URIBL_SBL autolearn=no version=3.3.2 spammy=fundamentals, seamlessly X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Apr 2018 20:37:12 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w36Kb6BO030970 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 6 Apr 2018 16:37:11 -0400 Received: by simark.ca (Postfix, from userid 112) id 484261E4C2; Fri, 6 Apr 2018 16:37:06 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id D67131E4C2; Fri, 6 Apr 2018 16:37:04 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Fri, 06 Apr 2018 20:37:00 -0000 From: Simon Marchi To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v2 3/5] Add gdb::string_view In-Reply-To: <01346239-a0d8-055f-7b6c-86ac48bfc7ac@redhat.com> References: <20180330214647.485-1-simon.marchi@polymtl.ca> <20180330214647.485-3-simon.marchi@polymtl.ca> <01346239-a0d8-055f-7b6c-86ac48bfc7ac@redhat.com> Message-ID: <7a85c51e05bada7f80daf413c0dd0e6c@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.4 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 6 Apr 2018 20:37:06 +0000 X-IsSubscribed: yes X-SW-Source: 2018-04/txt/msg00104.txt.bz2 Thanks for the comments. On 2018-04-06 14:08, Pedro Alves wrote: > On 03/30/2018 10:46 PM, Simon Marchi wrote: >> We had a few times the need for a data structure that does essentially >> what C++17's std::string_view does, which is to give an >> std::string-like >> interface (only the read-only operations) to an arbitrary character >> buffer. >> >> This patch adapts the files copied from libstdc++ by the previous >> patch >> to integrate them with GDB. Here's a summary of the changes: >> >> * Remove things related to wstring_view, u16string_view and >> u32string_view (I don't think we need them, but we can always add >> them >> later). > > Yeah, I don't think we'll ever need them. > >> >> * Remove usages of _GLIBCXX_BEGIN_NAMESPACE_VERSION and >> _GLIBCXX_END_NAMESPACE_VERSION. >> >> * Put the code in the gdb namespace. I had to add a few "std::" in >> front of std type usages. >> >> * Add a constructor that builds a string_view from an std::string, >> so >> that we can pass strings to string_view parameters seamlessly. >> Normally, that's handled by "operator __sv_type" in the std::string >> declaration, but it only exists when building with c++17. > > Yeah, that's how it was done when string_view was still part of > the C++ Library Fundamentals TS: > > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4480.html#string.view.cons > > I'd suggest taking a look at the diff between gcc/libstdc++'s: > > src/libstdc++-v3/include/experimental/string_view > src/libstdc++-v3/include/std/string_view > >> >> * Change __throw_out_of_range_fmt() for error(). >> >> * Make gdb::string_view and alias of std::string_view when building >> with >= c++17. > > s/and/an > >> >> * Remove a bunch of constexpr, because they are not valid in c++11 >> (e.g. they are not a single return line). > > You could instead pick the versions of those functions from the > experimental > string_view instead, which is written in C++11 constexpr form. E.g.: > > constexpr basic_string_view > - substr(size_type __pos, size_type __n=npos) const > + substr(size_type __pos, size_type __n = npos) const > noexcept(false) > { > - return __pos <= this->_M_len > - ? basic_string_view{this->_M_str + __pos, > - std::min(__n, size_type{this->_M_len - > __pos})} > - : > (__throw_out_of_range_fmt(__N("basic_string_view::substr: __pos " > - "(which is %zu) > > this->size() " > - "(which is %zu)"), > - __pos, this->size()), > basic_string_view{}); > + __pos = _M_check(__pos, "basic_string_view::substr"); > + const size_type __rlen = std::min(__n, _M_len - __pos); > + return basic_string_view{_M_str + __pos, __rlen}; > } Ah, from the name I thought that experimental version contained the code implementing future standards, before they were official... so I'll start from scratch using the version from experimental, does that sounds good? >> private: >> >> - static constexpr int >> + static int >> _S_compare(size_type __n1, size_type __n2) noexcept >> { >> const difference_type __diff = __n1 - __n2; >> @@ -428,11 +434,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> // argument participates in template argument deduction and the >> other >> // argument gets implicitly converted to the deduced type. See >> n3766.html. >> template >> - using __idt = common_type_t<_Tp>; >> + using __idt = typename std::common_type<_Tp>::type; >> } >> >> template >> - constexpr bool >> + bool >> operator==(basic_string_view<_CharT, _Traits> __x, >> basic_string_view<_CharT, _Traits> __y) noexcept >> { return __x.size() == __y.size() && __x.compare(__y) == 0; } >> @@ -541,8 +547,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> >> // [string.view.io], Inserters and extractors >> template >> - inline basic_ostream<_CharT, _Traits>& >> - operator<<(basic_ostream<_CharT, _Traits>& __os, >> + inline std::basic_ostream<_CharT, _Traits>& >> + operator<<(std::basic_ostream<_CharT, _Traits>& __os, >> basic_string_view<_CharT,_Traits> __str) >> { return __ostream_insert(__os, __str.data(), __str.size()); } >> >> @@ -550,13 +556,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> // basic_string_view typedef names >> >> using string_view = basic_string_view; >> -#ifdef _GLIBCXX_USE_WCHAR_T >> - using wstring_view = basic_string_view; >> -#endif >> -#ifdef _GLIBCXX_USE_C99_STDINT_TR1 >> - using u16string_view = basic_string_view; >> - using u32string_view = basic_string_view; >> -#endif >> >> // [string.view.hash], hash support: >> >> @@ -565,97 +564,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> >> template<> >> struct hash >> - : public __hash_base >> + : public std::__hash_base > > This looks suspiciously like using an internal implementation > detail of libstdc++. Where is std::__hash_base defined? > Does this compile with clang + libc++ ? Hmm indeed. Well, it looks like building with clang + libstdcxx fails. I'll make sure to test both of these combinations in the next iteration. Simon