[v3] Less noexcept
Marc Glisse
marc.glisse@inria.fr
Mon Sep 23 20:13:00 GMT 2013
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.glisse@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 Glisse
-------------- next part --------------
Index: 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)
{
More information about the Gcc-patches
mailing list