This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [v3] Less noexcept


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
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)
     {

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]