On Mon, 23 Sep 2013, Paolo Carlini wrote:
On 9/23/13 10:55 AM, Marc Glisse wrote:
Hello,
this patch was tested on x86_64 with a bootstrap and a simple make -k
check, without regression. Note that it doesn't completely fix 56166, it
merely admits that we may currently throw and avoids turning that into
std::terminate.
Of course.
Patch basically Ok, can you be a little less terse in the comments before the
noexcepts which you are removing and in fact must be there for conformance,
aren't just QoI? Point to an existing bug, when it exists, like 56166, add a
FIXME C++11 at least.
Like this?
It is funny that with fully dynamic strings, the copy constructor is
"better" than the move constructor: faster, doesn't throw, etc. I think we
should remove the move constructor in that case, or at least make it act
the same as the copy constructor. I didn't mark the copy constructor as
noexcept, but without checking the code it seems likely we could.
Also, __versa_string actually has the same issues as basic_string, if you
choose the reference counted base... I guess I have to do a patch to
remove noexcept there now :-( Has someone started work on some branch to
get a real C++11 basic_string, or are we waiting until the "lack of
manpower" argument convinces everyone to forget about trying to preserve
any ABI compatibility?
2013-09-24 Marc Glisse <marc.gli...@inria.fr>
PR libstdc++/58338
PR libstdc++/56166
* include/bits/basic_string.h (basic_string)
[basic_string(basic_string&&)]: Make the noexcept conditional.
[operator=(basic_string&&), assign(basic_string&&)]: Link to PR 58265.
[begin(), end(), rbegin(), rend(), clear]: Remove noexcept.
[pop_back]: Comment on the lack of noexcept.
* include/debug/string (basic_string) [basic_string(const _Allocator&),
basic_string(basic_string&&), begin(), end(), rbegin(), rend(), clear,
operator[](size_type), pop_back]: Comment out noexcept, until vstring
replaces basic_string.
--
Marc GlisseIndex: include/bits/basic_string.h
===================================================================
--- include/bits/basic_string.h (revision 202838)
+++ include/bits/basic_string.h (working copy)
@@ -502,21 +502,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
basic_string(size_type __n, _CharT __c, const _Alloc& __a = _Alloc());
#if __cplusplus >= 201103L
/**
* @brief Move construct string.
* @param __str Source string.
*
* The newly-created string contains the exact contents of @a __str.
* @a __str is a valid, but unspecified string.
**/
- basic_string(basic_string&& __str) noexcept
+ basic_string(basic_string&& __str)
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
+ noexcept // FIXME C++11: should always be noexcept.
+#endif
: _M_dataplus(__str._M_dataplus)
{
#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
__str._M_data(_S_empty_rep()._M_refdata());
#else
__str._M_data(_S_construct(size_type(), _CharT(), get_allocator()));
#endif
}
/**
@@ -574,20 +577,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
#if __cplusplus >= 201103L
/**
* @brief Move assign the value of @a str to this string.
* @param __str Source string.
*
* The contents of @a str are moved into this string (without copying).
* @a str is a valid, but unspecified string.
**/
+ // PR 58265, this should be noexcept.
basic_string&
operator=(basic_string&& __str)
{
// NB: DR 1204.
this->swap(__str);
return *this;
}
/**
* @brief Set value to string constructed from initializer %list.
@@ -600,78 +604,78 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return *this;
}
#endif // C++11
// Iterators:
/**
* Returns a read/write iterator that points to the first character in
* the %string. Unshares the string.
*/
iterator
- begin() _GLIBCXX_NOEXCEPT
+ begin() // FIXME C++11: should be noexcept.
{
_M_leak();
return iterator(_M_data());
}
/**
* Returns a read-only (constant) iterator that points to the first
* character in the %string.
*/
const_iterator
begin() const _GLIBCXX_NOEXCEPT
{ return const_iterator(_M_data()); }
/**
* Returns a read/write iterator that points one past the last
* character in the %string. Unshares the string.
*/
iterator
- end() _GLIBCXX_NOEXCEPT
+ end() // FIXME C++11: should be noexcept.
{
_M_leak();
return iterator(_M_data() + this->size());
}
/**
* Returns a read-only (constant) iterator that points one past the
* last character in the %string.
*/
const_iterator
end() const _GLIBCXX_NOEXCEPT
{ return const_iterator(_M_data() + this->size()); }
/**
* Returns a read/write reverse iterator that points to the last
* character in the %string. Iteration is done in reverse element
* order. Unshares the string.
*/
reverse_iterator
- rbegin() _GLIBCXX_NOEXCEPT
+ rbegin() // FIXME C++11: should be noexcept.
{ return reverse_iterator(this->end()); }
/**
* Returns a read-only (constant) reverse iterator that points
* to the last character in the %string. Iteration is done in
* reverse element order.
*/
const_reverse_iterator
rbegin() const _GLIBCXX_NOEXCEPT
{ return const_reverse_iterator(this->end()); }
/**
* Returns a read/write reverse iterator that points to one before the
* first character in the %string. Iteration is done in reverse
* element order. Unshares the string.
*/
reverse_iterator
- rend() _GLIBCXX_NOEXCEPT
+ rend() // FIXME C++11: should be noexcept.
{ return reverse_iterator(this->begin()); }
/**
* Returns a read-only (constant) reverse iterator that points
* to one before the first character in the %string. Iteration
* is done in reverse element order.
*/
const_reverse_iterator
rend() const _GLIBCXX_NOEXCEPT
{ return const_reverse_iterator(this->begin()); }
@@ -799,21 +803,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* data.
*/
void
reserve(size_type __res_arg = 0);
/**
* Erases the string, making it empty.
*/
// PR 56166: this should not throw.
void
- clear() _GLIBCXX_NOEXCEPT
+ clear()
{ _M_mutate(0, this->size(), 0); }
/**
* Returns true if the %string is empty. Equivalent to
* <code>*this == ""</code>.
*/
bool
empty() const _GLIBCXX_NOEXCEPT
{ return this->size() == 0; }
@@ -1081,20 +1085,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#if __cplusplus >= 201103L
/**
* @brief Set value to contents of another string.
* @param __str Source string to use.
* @return Reference to this string.
*
* This function sets this string to the exact contents of @a __str.
* @a __str is a valid, but unspecified string.
*/
+ // PR 58265, this should be noexcept.
basic_string&
assign(basic_string&& __str)
{
this->swap(__str);
return *this;
}
#endif // C++11
/**
* @brief Set value to a substring of a string.
@@ -1410,21 +1415,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
iterator
erase(iterator __first, iterator __last);
#if __cplusplus >= 201103L
/**
* @brief Remove the last character.
*
* The string must be non-empty.
*/
void
- pop_back()
+ pop_back() // FIXME C++11: should be noexcept.
{ erase(size()-1, 1); }
#endif // C++11
/**
* @brief Replace characters with value from another string.
* @param __pos Index of first character to replace.
* @param __n Number of characters to be replaced.
* @param __str String to insert.
* @return Reference to this string.
* @throw std::out_of_range If @a pos is beyond the end of this
Index: include/debug/string
===================================================================
--- include/debug/string (revision 202838)
+++ include/debug/string (working copy)
@@ -63,21 +63,21 @@ namespace __gnu_debug
typedef __gnu_debug::_Safe_iterator<typename _Base::const_iterator,
basic_string> const_iterator;
typedef std::reverse_iterator<iterator> reverse_iterator;
typedef std::reverse_iterator<const_iterator> const_reverse_iterator;
using _Base::npos;
// 21.3.1 construct/copy/destroy:
explicit basic_string(const _Allocator& __a = _Allocator())
- _GLIBCXX_NOEXCEPT
+ // _GLIBCXX_NOEXCEPT
: _Base(__a)
{ }
// Provides conversion from a release-mode string to a debug-mode string
basic_string(const _Base& __base) : _Base(__base) { }
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 42. string ctors specify wrong default allocator
basic_string(const basic_string& __str)
: _Base(__str, 0, _Base::npos, __str.get_allocator())
@@ -107,21 +107,21 @@ namespace __gnu_debug
template<typename _InputIterator>
basic_string(_InputIterator __begin, _InputIterator __end,
const _Allocator& __a = _Allocator())
: _Base(__gnu_debug::__base(__gnu_debug::__check_valid_range(__begin,
__end)),
__gnu_debug::__base(__end), __a)
{ }
#if __cplusplus >= 201103L
- basic_string(basic_string&& __str) noexcept
+ basic_string(basic_string&& __str) // noexcept
: _Base(std::move(__str))
{ }
basic_string(std::initializer_list<_CharT> __l,
const _Allocator& __a = _Allocator())
: _Base(__l, __a)
{ }
#endif // C++11
~basic_string() _GLIBCXX_NOEXCEPT { }
@@ -165,45 +165,45 @@ namespace __gnu_debug
operator=(std::initializer_list<_CharT> __l)
{
*static_cast<_Base*>(this) = __l;
this->_M_invalidate_all();
return *this;
}
#endif // C++11
// 21.3.2 iterators:
iterator
- begin() _GLIBCXX_NOEXCEPT
+ begin() // _GLIBCXX_NOEXCEPT
{ return iterator(_Base::begin(), this); }
const_iterator
begin() const _GLIBCXX_NOEXCEPT
{ return const_iterator(_Base::begin(), this); }
iterator
- end() _GLIBCXX_NOEXCEPT
+ end() // _GLIBCXX_NOEXCEPT
{ return iterator(_Base::end(), this); }
const_iterator
end() const _GLIBCXX_NOEXCEPT
{ return const_iterator(_Base::end(), this); }
reverse_iterator
- rbegin() _GLIBCXX_NOEXCEPT
+ rbegin() // _GLIBCXX_NOEXCEPT
{ return reverse_iterator(end()); }
const_reverse_iterator
rbegin() const _GLIBCXX_NOEXCEPT
{ return const_reverse_iterator(end()); }
reverse_iterator
- rend() _GLIBCXX_NOEXCEPT
+ rend() // _GLIBCXX_NOEXCEPT
{ return reverse_iterator(begin()); }
const_reverse_iterator
rend() const _GLIBCXX_NOEXCEPT
{ return const_reverse_iterator(begin()); }
#if __cplusplus >= 201103L
const_iterator
cbegin() const noexcept
{ return const_iterator(_Base::begin(), this); }
@@ -251,42 +251,42 @@ namespace __gnu_debug
__catch(...)
{ }
}
}
#endif
using _Base::capacity;
using _Base::reserve;
void
- clear() _GLIBCXX_NOEXCEPT
+ clear() // _GLIBCXX_NOEXCEPT
{
_Base::clear();
this->_M_invalidate_all();
}
using _Base::empty;
// 21.3.4 element access:
const_reference
operator[](size_type __pos) const _GLIBCXX_NOEXCEPT
{
_GLIBCXX_DEBUG_VERIFY(__pos <= this->size(),
_M_message(__gnu_debug::__msg_subscript_oob)
._M_sequence(*this, "this")
._M_integer(__pos, "__pos")
._M_integer(this->size(), "size"));
return _M_base()[__pos];
}
reference
- operator[](size_type __pos) _GLIBCXX_NOEXCEPT
+ operator[](size_type __pos) // _GLIBCXX_NOEXCEPT
{
#ifdef _GLIBCXX_DEBUG_PEDANTIC
__glibcxx_check_subscript(__pos);
#else
// as an extension v3 allows s[s.size()] when s is non-const.
_GLIBCXX_DEBUG_VERIFY(__pos <= this->size(),
_M_message(__gnu_debug::__msg_subscript_oob)
._M_sequence(*this, "this")
._M_integer(__pos, "__pos")
._M_integer(this->size(), "size"));
@@ -576,21 +576,21 @@ namespace __gnu_debug
// 151. can't currently clear() empty container
__glibcxx_check_erase_range(__first, __last);
typename _Base::iterator __res = _Base::erase(__first.base(),
__last.base());
this->_M_invalidate_all();
return iterator(__res, this);
}
#if __cplusplus >= 201103L
void
- pop_back() noexcept
+ pop_back() // noexcept
{
__glibcxx_check_nonempty();
_Base::pop_back();
this->_M_invalidate_all();
}
#endif // C++11
basic_string&
replace(size_type __pos1, size_type __n1, const basic_string& __str)
{
