[libstdc++-v3] PR 59529, 59530, 59531 Was(Re: Implement N3762 string_view: a non-owning reference to a string.)
Ed Smith-Rowland
3dw4rd@verizon.net
Sun Dec 22 01:56:00 GMT 2013
On 12/17/2013 11:06 AM, Peter A. Bigot wrote:
> On 11/20/2013 08:09 AM, Ed Smith-Rowland wrote:
>> On 11/18/2013 09:08 AM, Daniel Krügler wrote:
>>> 2013/11/15 Ed Smith-Rowland <3dw4rd@verizon.net>:
>>>> Greetings,
>>>>
>>>> As the title says. It's a pretty simple class" a non-mutating
>>>> read-only
>>>> view into a const CharT* or a basic_string.
>>>>
>>>> Built and tested on x86_64-linux.
>>>>
>>>> OK?
>>> Sorry, if this is documented somewhere, but is this intended to
>>> implement
>>>
>>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3762.html
>>>
>>> ? I don't think that your implementation is valid, because it maps an
>>> empty string view into a NULL data pointer. But according to the
>>> specification of data() the return type, this isn't feasible:
>>>
>>> <quote>
>>> constexpr const charT* data() const noexcept;
>>>
>>> Returns: A non-null pointer p such that p + i == &operator[](i) for
>>> each i in [0,size()).
>>> </quote>
>>>
>>> - Daniel
>>>
>> Right.
>>
>> Here is a fix that introduces an constexpr unit length string to
>> represent empty strings so that refs to empty strings don't point to the
>> ether.
>>
>
> Thanks for providing this feature; I'm finding it very useful.
>
> There's a small problem with this fix in that some zero-length
> string_views should remain pointing into the referenced object, and not
> be remapped to the constexpr one. This showed up after fixing a boundary
> condition problem with substr, recorded in:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59529
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59530 notes a similar
> problem with operator[], and
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59531 fixes a buffer overrun
> in copy(). Proposed patches are attached to each issue.
>
> Peter
>
I've looked over these patches (thanks Peter!) and tweaked up a couple
patches (no need to leave commented out tests, and copy can start from
pos=length()).
I'm finishing testing on x86_64-linux.
Paolo, are there any Copyright issues we need to be aware of? Does
Peter need a Copyright form? These are fairly obvious one-liners I
think. There are no real alternatives to these patches. What ChangeLog
attribution is appropriate?
Ed
-------------- next part --------------
2013-12-21 Edward Smith-Rowland <3dw4rd@verizon.net>
PR libstdc++/59529
PR libstdc++/59530
PR libstdc++/59531
* include/experimental/string_view
(basic_string_view(const _CharT*, size_type)): Don't use static
constexpr string for zero length but non-null pointer.
(substr(size_type, size_type)): Zero length substring starting at end
is valid.
(operator[]()): Access at size() is illegal (No null terminator).
(copy): fix terminating loop cndition. Copy from pos = size() is legal.
* testsuite/experimental/string_view/operations/substr/char/1.cc: Don't
throw when pos == length.
* testsuite/experimental/string_view/operations/substr/wchar_t/1.cc:
Ditto.
* testsuite/experimental/string_view/element_access/char/1.cc: Access at
sv[size()] is illegal.
* testsuite/experimental/string_view/element_access/wchar_t/1.cc: Ditto.
* testsuite/experimental/string_view/operations/copy/char/1.cc: New.
* testsuite/experimental/string_view/operations/copy/wchar_t/1.cc: New.
-------------- next part --------------
Index: include/experimental/string_view
===================================================================
--- include/experimental/string_view (revision 206165)
+++ include/experimental/string_view (working copy)
@@ -117,7 +117,7 @@
constexpr basic_string_view(const _CharT* __str, size_type __len)
: _M_len{__str == nullptr ? 0 :__len},
- _M_str{__str == nullptr || __len == 0 ? _S_empty_str : __str}
+ _M_str{__str == nullptr ? _S_empty_str : __str}
{ }
basic_string_view&
@@ -181,7 +181,7 @@
constexpr const _CharT&
operator[](size_type __pos) const
{
- _GLIBCXX_DEBUG_ASSERT(__pos <= this->_M_len);
+ _GLIBCXX_DEBUG_ASSERT(__pos < this->_M_len);
return *(this->_M_str + __pos);
}
@@ -256,14 +256,14 @@
copy(_CharT* __str, size_type __n, size_type __pos = 0) const
{
__glibcxx_requires_string_len(__str, __n);
- if (__pos >= this->_M_len)
+ if (__pos > this->_M_len)
__throw_out_of_range_fmt(__N("basic_string_view::at: __pos "
"(which is %zu) >= this->size() "
"(which is %zu)"),
__pos, this->size());
size_type __rlen{std::min(__n, size_type{this->_M_len - __pos})};
for (auto __begin = this->_M_str + __pos,
- __end = this->_M_str + __rlen; __begin != __end;)
+ __end = __begin + __rlen; __begin != __end;)
*__str++ = *__begin++;
return __rlen;
}
@@ -274,7 +274,7 @@
constexpr basic_string_view
substr(size_type __pos, size_type __n=npos) const
{
- return __pos < this->_M_len
+ 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::at: __pos "
Index: testsuite/experimental/string_view/operations/substr/char/1.cc
===================================================================
--- testsuite/experimental/string_view/operations/substr/char/1.cc (revision 206165)
+++ testsuite/experimental/string_view/operations/substr/char/1.cc (working copy)
@@ -63,11 +63,9 @@
{
str02 = str01.substr(csz01);
VERIFY( str02.size() == 0 );
+ VERIFY( str02.begin() == str01.end() );
+ VERIFY( true );
}
- catch(std::out_of_range& fail)
- {
- VERIFY( true ); // No terminating null in basic_string_view
- }
catch(...)
{
VERIFY( false );
Index: testsuite/experimental/string_view/operations/substr/wchar_t/1.cc
===================================================================
--- testsuite/experimental/string_view/operations/substr/wchar_t/1.cc (revision 206165)
+++ testsuite/experimental/string_view/operations/substr/wchar_t/1.cc (working copy)
@@ -63,11 +63,9 @@
{
str02 = str01.substr(csz01);
VERIFY( str02.size() == 0 );
+ VERIFY( str02.begin() == str01.end() );
+ VERIFY( true );
}
- catch(std::out_of_range& fail)
- {
- VERIFY( true ); // No terminating null in basic_string_view
- }
catch(...)
{
VERIFY( false );
Index: testsuite/experimental/string_view/element_access/char/1.cc
===================================================================
--- testsuite/experimental/string_view/element_access/char/1.cc (revision 206165)
+++ testsuite/experimental/string_view/element_access/char/1.cc (working copy)
@@ -41,8 +41,9 @@
csz01 = str01.size();
cref cref1 = str01[csz01 - 1];
VERIFY( cref1 == 'a' );
- cref cref2 = str01[csz01];
- VERIFY( cref2 == char() );
+ // Undefined behavior at size().
+ //cref cref2 = str01[csz01];
+ //VERIFY( cref2 == char() );
// const_reference at(size_type pos) const;
csz01 = str01.size();
Index: testsuite/experimental/string_view/element_access/wchar_t/1.cc
===================================================================
--- testsuite/experimental/string_view/element_access/wchar_t/1.cc (revision 206165)
+++ testsuite/experimental/string_view/element_access/wchar_t/1.cc (working copy)
@@ -41,8 +41,9 @@
csz01 = str01.size();
cref cref1 = str01[csz01 - 1];
VERIFY( cref1 == L'a' );
- cref cref2 = str01[csz01];
- VERIFY( cref2 == wchar_t() );
+ // Undefined behavior at size().
+ //cref cref2 = str01[csz01];
+ //VERIFY( cref2 == wchar_t() );
// const_reference at(size_type pos) const;
csz01 = str01.size();
Index: testsuite/experimental/string_view/operations/copy/char/1.cc
===================================================================
--- testsuite/experimental/string_view/operations/copy/char/1.cc (revision 0)
+++ testsuite/experimental/string_view/operations/copy/char/1.cc (working copy)
@@ -0,0 +1,50 @@
+// { dg-options "-std=gnu++1y" }
+
+// Copyright (C) 2013 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// basic_string_view::copy
+
+#include <experimental/string_view>
+#include <stdexcept>
+#include <testsuite_hooks.h>
+
+bool
+test01()
+{
+ bool test [[gnu::unused]] = true;
+
+ typedef std::experimental::string_view::size_type csize_type;
+
+ const char str_lit01[] = "123456789A";
+ const std::experimental::string_view str01(str_lit01);
+ char buffer[4] = { 0 };
+
+ csize_type len = str01.copy(buffer, sizeof(buffer), 8);
+ VERIFY( 2 == len );
+ VERIFY( '9' == buffer[0] );
+
+ return test;
+}
+
+int
+main()
+{
+ test01();
+
+ return 0;
+}
Index: testsuite/experimental/string_view/operations/copy/wchar_t/1.cc
===================================================================
--- testsuite/experimental/string_view/operations/copy/wchar_t/1.cc (revision 0)
+++ testsuite/experimental/string_view/operations/copy/wchar_t/1.cc (working copy)
@@ -0,0 +1,51 @@
+// { dg-options "-std=gnu++1y" }
+
+// Copyright (C) 2013 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// basic_string_view::copy
+
+#include <experimental/string_view>
+#include <stdexcept>
+#include <testsuite_hooks.h>
+
+bool
+test01()
+{
+ bool test [[gnu::unused]] = true;
+
+ typedef std::experimental::wstring_view::size_type csize_type;
+ csize_type csz01;
+
+ const wchar_t str_lit01[] = L"123456789A";
+ const std::experimental::wstring_view str01(str_lit01);
+ wchar_t buffer[4] = { 0 };
+
+ csize_type len = str01.copy(buffer, sizeof(buffer), 8);
+ VERIFY( 2 == len );
+ VERIFY( L'9' == buffer[0] );
+
+ return test;
+}
+
+int
+main()
+{
+ test01();
+
+ return 0;
+}
More information about the Libstdc++
mailing list