Changes to std::match_results and <regex>
Jonathan Wakely
jwakely.gcc@gmail.com
Sun Jun 6 14:55:00 GMT 2010
Our match_results has some problems:
size() returns N+1 where N is the number of elements, which already
includes the 0th sub-match (i.e. the entire match) so the returned
value is off-by-one. It should be n+1 where n is the number of marked
sub-expressions.
operator[] does not check for sub >= size() so allows out-of-range accesses.
position() can result in undefined behaviour if called with an
out-of-range argument because it will try to compare a potentially
singular iterator. This is really a problem with the draft standard,
which doesn't make it clear that a match_results can be singular and
there are implied preconditions on some member functions. I plan to
file an NB comment about this.
_M_prefix and _M_suffix could be stored in the vector, reducing the
size of an unmatched match_results. This also makes _M_matched
unnecessary because !_M_matched is implied by the vector being empty.
This reduces the size of match_results by 4 pointers and 3 bools and
simplifies copying, moving and swapping because the only state is in
the vector.
Finally, it's missing move operations and other changes since TR1.
The patch below addresses all these issues, so I would like to check
it in. It changes the ABI of match_results, but as that class is
completely non-functional it can't break any programs unless they
explicitly make use of sizeof(match_results) but don't actually
instantiate match_results.
My suggested implementation is documented in the code:
The vector base is empty if this does not represent a successful match.
Otherwise it contains n+3 elements where n is the number of marked
sub-expressions:
[0] entire match
[1] 1st marked subexpression
...
[n] nth marked subexpression
[n+1] prefix
[n+2] suffix
This means size() == n+1 == (N ? N-2 : 0)
Copying, moving and swapping, as well as empty(), are now simply
forwarded to the vector base.
I made position() return -1 for out-of-range, consistent with
boost::regex (and with e.g. string::find)
operator[] checks its argument and returns a static object
representing an unmatched subexpression (as required).
Does anyone have any objections or improvements to this change?
-------------- next part --------------
Index: include/tr1_impl/regex
===================================================================
--- include/tr1_impl/regex (revision 160340)
+++ include/tr1_impl/regex (working copy)
@@ -789,12 +789,24 @@ namespace regex_constants
* @brief Copy-constructs a basic regular expression.
*
* @param rhs A @p regex object.
- */
+ */
basic_regex(const basic_regex& __rhs)
: _M_flags(__rhs._M_flags), _M_pattern(__rhs._M_pattern),
_M_mark_count(__rhs._M_mark_count)
{ _M_compile(); }
+#ifdef _GLIBCXX_INCLUDE_AS_CXX0X
+ /**
+ * @brief Move-constructs a basic regular expression.
+ *
+ * @param rhs A @p regex object.
+ */
+ basic_regex(basic_regex&& __rhs)
+ : _M_flags(__rhs._M_flags), _M_pattern(std::move(__rhs._M_pattern)),
+ _M_mark_count(__rhs._M_mark_count)
+ { _M_compile(); }
+#endif
+
/**
* @brief Constructs a basic regular expression from the string
* @p s interpreted according to the flags in @p f.
@@ -858,6 +870,15 @@ namespace regex_constants
operator=(const basic_regex& __rhs)
{ return this->assign(__rhs); }
+#ifdef _GLIBCXX_INCLUDE_AS_CXX0X
+ /**
+ * @brief Move-assigns one regular expression to another.
+ */
+ basic_regex&
+ operator=(basic_regex&& __rhs)
+ { return this->assign(std::move(__rhs)); }
+#endif
+
/**
* @brief Replaces a regular expression with a new one constructed from
* a C-style null-terminated string.
@@ -875,9 +896,9 @@ namespace regex_constants
*
* @param A pointer to a string containing a regular expression.
*/
- template<typename _Ch_typeraits, typename _Allocator>
+ template<typename _Traits, typename _Allocator>
basic_regex&
- operator=(const basic_string<_Ch_type, _Ch_typeraits, _Allocator>& __s)
+ operator=(const basic_string<_Ch_type, _Traits, _Allocator>& __s)
{ return this->assign(__s, flags()); }
// [7.8.3] assign
@@ -893,7 +914,21 @@ namespace regex_constants
this->swap(__tmp);
return *this;
}
-
+
+#ifdef _GLIBCXX_INCLUDE_AS_CXX0X
+ /**
+ * @brief the real move-assignment operator.
+ *
+ * @param that Another regular expression object.
+ */
+ basic_regex&
+ assign(basic_regex&& __that)
+ {
+ basic_regex(std::move(__that)).swap(*this);
+ return *this;
+ }
+#endif
+
/**
* @brief Assigns a new regular expression to a regex object from a
* C-style null-terminated string containing a regular expression
@@ -940,9 +975,9 @@ namespace regex_constants
* pattern interpreted according to @p flags. If regex_error is thrown,
* *this remains unchanged.
*/
- template<typename _Ch_typeraits, typename _Allocator>
+ template<typename _Traits, typename _Allocator>
basic_regex&
- assign(const basic_string<_Ch_type, _Ch_typeraits, _Allocator>& __s,
+ assign(const basic_string<_Ch_type, _Traits, _Allocator>& __s,
flag_type __f = regex_constants::ECMAScript)
{
basic_regex __tmp(__s, __f);
@@ -1092,6 +1127,9 @@ namespace regex_constants
typedef typename iterator_traits<_BiIter>::difference_type
difference_type;
typedef _BiIter iterator;
+#ifdef _GLIBCXX_INCLUDE_AS_CXX0X
+ typedef basic_string<value_type> string_type;
+#endif
public:
bool matched;
@@ -1737,6 +1775,14 @@ namespace regex_constants
const sub_match<_Bi_iter>& __m)
{ return __os << __m.str(); }
+ template<typename _Bi_iter>
+ inline const sub_match<_Bi_iter>&
+ __unmatched_sub()
+ {
+ static const sub_match<_Bi_iter> __unmatched = sub_match<_Bi_iter>();
+ return __unmatched;
+ }
+
// [7.10] Class template match_results
/**
* @brief The results of a match or search operation.
@@ -1766,6 +1812,17 @@ namespace regex_constants
: private std::vector<std::_GLIBCXX_TR1 sub_match<_Bi_iter>, _Allocator>
{
private:
+ /*
+ The vector base is empty if this does not represent a successful match.
+ Otherwise it contains n+3 elements where n is the number of marked
+ sub-expressions:
+ [0] entire match
+ [1] 1st marked subexpression
+ ...
+ [n] nth marked subexpression
+ [n+1] prefix
+ [n+2] suffix
+ */
typedef std::vector<std::_GLIBCXX_TR1 sub_match<_Bi_iter>, _Allocator>
_Base_type;
@@ -1775,13 +1832,19 @@ namespace regex_constants
*/
//@{
typedef sub_match<_Bi_iter> value_type;
+#ifdef _GLIBCXX_INCLUDE_AS_CXX0X
+ typedef const value_type& const_reference;
+ /* TODO: needs allocator_traits */
+ typedef typename _Allocator::size_type size_type;
+#else
typedef typename _Allocator::const_reference const_reference;
+ typedef typename _Allocator::size_type size_type;
+#endif
typedef const_reference reference;
typedef typename _Base_type::const_iterator const_iterator;
typedef const_iterator iterator;
typedef typename iterator_traits<_Bi_iter>::difference_type
difference_type;
- typedef typename _Allocator::size_type size_type;
typedef _Allocator allocator_type;
typedef typename iterator_traits<_Bi_iter>::value_type char_type;
typedef basic_string<char_type> string_type;
@@ -1799,15 +1862,14 @@ namespace regex_constants
*/
explicit
match_results(const _Allocator& __a = _Allocator())
- : _Base_type(__a), _M_matched(false)
+ : _Base_type(__a)
{ }
/**
* @brief Copy constructs a %match_results.
*/
match_results(const match_results& __rhs)
- : _Base_type(__rhs), _M_matched(__rhs._M_matched),
- _M_prefix(__rhs._M_prefix), _M_suffix(__rhs._M_suffix)
+ : _Base_type(__rhs)
{ }
/**
@@ -1816,11 +1878,29 @@ namespace regex_constants
match_results&
operator=(const match_results& __rhs)
{
- match_results __tmp(__rhs);
- this->swap(__tmp);
+ match_results(__rhs).swap(*this);
return *this;
}
+#ifdef _GLIBCXX_INCLUDE_AS_CXX0X
+ /**
+ * @brief Move constructs a %match_results.
+ */
+ match_results(match_results&& __rhs)
+ : _Base_type(std::forward<_Base_type>(__rhs))
+ { }
+
+ /**
+ * @brief Move assigns rhs to *this.
+ */
+ match_results&
+ operator=(match_results&& __rhs)
+ {
+ match_results(std::move(__rhs)).swap(*this);
+ return *this;
+ }
+#endif
+
/**
* @brief Destroys a %match_results object.
*/
@@ -1845,7 +1925,10 @@ namespace regex_constants
*/
size_type
size() const
- { return _M_matched ? _Base_type::size() + 1 : 0; }
+ {
+ size_type __size = _Base_type::size();
+ return __size ? __size - 2 : 0;
+ }
//size_type
//max_size() const;
@@ -1858,7 +1941,7 @@ namespace regex_constants
*/
bool
empty() const
- { return size() == 0; }
+ { return _Base_type::empty(); }
//@}
@@ -1876,7 +1959,7 @@ namespace regex_constants
*/
difference_type
length(size_type __sub = 0) const
- { return _M_matched ? this->str(__sub).length() : 0; }
+ { return (*this)[__sub].length(); }
/**
* @brief Gets the offset of the beginning of the indicated submatch.
@@ -1891,8 +1974,8 @@ namespace regex_constants
difference_type
position(size_type __sub = 0) const
{
- return _M_matched ? std::distance(this->prefix().first,
- (*this)[__sub].first) : 0;
+ return __sub < size() ? std::distance(this->prefix().first,
+ (*this)[__sub].first) : -1;
}
/**
@@ -1904,7 +1987,7 @@ namespace regex_constants
*/
string_type
str(size_type __sub = 0) const
- { return _M_matched ? (*this)[__sub].str() : string_type(); }
+ { return (*this)[__sub].str(); }
/**
* @brief Gets a %sub_match reference for the match or submatch.
@@ -1918,7 +2001,10 @@ namespace regex_constants
*/
const_reference
operator[](size_type __sub) const
- { return _Base_type::operator[](__sub); }
+ {
+ return __sub < size() ? _Base_type::operator[](__sub)
+ : __unmatched_sub<_Bi_iter>();
+ }
/**
* @brief Gets a %sub_match representing the match prefix.
@@ -1929,7 +2015,10 @@ namespace regex_constants
*/
const_reference
prefix() const
- { return _M_prefix; }
+ {
+ return !empty() ? _Base_type::operator[](_Base_type::size() - 2)
+ : __unmatched_sub<_Bi_iter>();
+ }
/**
* @brief Gets a %sub_match representing the match suffix.
@@ -1940,7 +2029,10 @@ namespace regex_constants
*/
const_reference
suffix() const
- { return _M_suffix; }
+ {
+ return !empty() ? _Base_type::operator[](_Base_type::size() - 1)
+ : __unmatched_sub<_Bi_iter>();
+ }
/**
* @brief Gets an iterator to the start of the %sub_match collection.
@@ -1986,6 +2078,46 @@ namespace regex_constants
*/
//@{
+#ifdef _GLIBCXX_INCLUDE_AS_CXX0X
+ /**
+ * @todo Implement this function.
+ */
+ template <class _Out_iter>
+ _Out_iter
+ format(_Out_iter __out,
+ const char_type* __fmt_first, const char_type* __fmt_last,
+ regex_constants::match_flag_type __flags
+ = regex_constants::format_default) const;
+
+ /**
+ * @todo Implement this function.
+ */
+ template <class _Out_iter, class _Str_traits, class _Str_Alloc>
+ _Out_iter
+ format(_Out_iter __out,
+ const basic_string<char_type, _Str_traits, _Str_Alloc>& __fmt,
+ regex_constants::match_flag_type __flags
+ = regex_constants::format_default) const;
+
+ /**
+ * @todo Implement this function.
+ */
+ template <class _Str_traits, class _Str_Alloc>
+ basic_string<char_type, _Str_traits, _Str_Alloc>
+ format(const basic_string<char_type, _Str_traits, _Str_Alloc>& __fmt,
+ regex_constants::match_flag_type __flags
+ = regex_constants::format_default) const;
+
+ /**
+ * @todo Implement this function.
+ */
+ string_type
+ format(const char_type* __fmt,
+ regex_constants::match_flag_type __flags
+ = regex_constants::format_default) const;
+
+#else
+
/**
* @todo Implement this function.
*/
@@ -2002,6 +2134,7 @@ namespace regex_constants
format(const string_type& __fmt,
regex_constants::match_flag_type __flags
= regex_constants::format_default) const;
+#endif
//@}
@@ -2029,20 +2162,10 @@ namespace regex_constants
*/
void
swap(match_results& __that)
- {
- _Base_type::swap(__that);
- std::swap(_M_matched, __that._M_matched);
- std::swap(_M_prefix, __that._M_prefix);
- std::swap(_M_suffix, __that._M_suffix);
- }
+ { _Base_type::swap(__that); }
//@}
-
- private:
- bool _M_matched;
- value_type _M_prefix;
- value_type _M_suffix;
};
-
+
typedef match_results<const char*> cmatch;
typedef match_results<string::const_iterator> smatch;
#ifdef _GLIBCXX_USE_WCHAR_T
More information about the Libstdc++
mailing list