diff mbox

[v3] Less noexcept

Message ID alpine.DEB.2.10.1309231727470.4526@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse Sept. 23, 2013, 3:55 p.m. UTC
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.

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.
 	* 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.

Comments

Paolo Carlini Sept. 23, 2013, 4:29 p.m. UTC | #1
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.

Thanks,
Paolo.
diff mbox

Patch

Index: include/bits/basic_string.h
===================================================================
--- include/bits/basic_string.h	(revision 202831)
+++ 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
+#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() // Can throw.
       {
 	_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() // Can throw.
       {
 	_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() // Can throw.
       { 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() // Can throw.
       { 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.
Index: include/debug/string
===================================================================
--- include/debug/string	(revision 202831)
+++ 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)
     {