Fix incorrect results from std::boyer_moore_searcher

Message ID 20170418110938.GA29714@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely April 18, 2017, 11:09 a.m.
Nico noticed the return value of std::boyer_moore_searcher has the
wrong position for the end of the match. Fixed by this patch, which
also does some minor cleanup (like removing redundant std:: quals that
were copied from the std::experimental::boyer_moore_searcher version).

	* include/std/functional (default_searcher, __boyer_moore_array_base)
	(__is_std_equal_to, __boyer_moore_base_t, boyer_moore_searcher)
	(boyer_moore_horspool_searcher): Remove redundant namespace
	qualification.
	(default_searcher::operator()): Construct return value early and
	advance second member in-place.
	(boyer_moore_horspool_searcher::operator()): Increment random access
	iterator directly instead of using std::next.
	(boyer_moore_searcher::operator()): Fix return value.
	* testsuite/20_util/function_objects/searchers.cc: Check both parts
	of return values.

Tested ppc64le-linux, committed to trunk.
commit 35bccf225502eaa895321500b20e432605119b93
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Apr 18 11:51:35 2017 +0100

    Fix incorrect results from std::boyer_moore_searcher
    
    	* include/std/functional (default_searcher, __boyer_moore_array_base)
    	(__is_std_equal_to, __boyer_moore_base_t, boyer_moore_searcher)
    	(boyer_moore_horspool_searcher): Remove redundant namespace
    	qualification.
    	(default_searcher::operator()): Construct return value early and
    	advance second member in-place.
    	(boyer_moore_horspool_searcher::operator()): Increment random access
    	iterator directly instead of using std::next.
    	(boyer_moore_searcher::operator()): Fix return value.
    	* testsuite/20_util/function_objects/searchers.cc: Check both parts
    	of return values.

Patch

diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional
index 0661253..e4a82ee 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -969,17 +969,17 @@  _GLIBCXX_MEM_FN_TRAITS(&&, false_type, true_type)
 	operator()(_ForwardIterator2 __first, _ForwardIterator2 __last) const
 	{
 	  _ForwardIterator2 __first_ret =
-	    std::search(__first, __last,
-			std::get<0>(_M_m), std::get<1>(_M_m),
+	    std::search(__first, __last, std::get<0>(_M_m), std::get<1>(_M_m),
 			std::get<2>(_M_m));
-	  _ForwardIterator2 __second_ret = __first_ret == __last ?
-	    __last :  std::next(__first_ret, std::distance(std::get<0>(_M_m),
-							   std::get<1>(_M_m)));
-	  return std::make_pair(__first_ret, __second_ret);
+	  auto __ret = std::make_pair(__first_ret, __first_ret);
+	  if (__ret.first != __last)
+	    std::advance(__ret.second, std::distance(std::get<0>(_M_m),
+						     std::get<1>(_M_m)));
+	  return __ret;
 	}
 
     private:
-      std::tuple<_ForwardIterator1, _ForwardIterator1, _BinaryPredicate> _M_m;
+      tuple<_ForwardIterator1, _ForwardIterator1, _BinaryPredicate> _M_m;
     };
 
   template<typename _Key, typename _Tp, typename _Hash, typename _Pred>
@@ -1025,7 +1025,7 @@  _GLIBCXX_MEM_FN_TRAITS(&&, false_type, true_type)
 	    for (__diff_type __i = 0; __i < __patlen - 1; ++__i)
 	      {
 		auto __ch = __pat[__i];
-		using _UCh = std::make_unsigned_t<decltype(__ch)>;
+		using _UCh = make_unsigned_t<decltype(__ch)>;
 		auto __uch = static_cast<_UCh>(__ch);
 		std::get<0>(_M_bad_char)[__uch] = __patlen - 1 - __i;
 	      }
@@ -1037,7 +1037,7 @@  _GLIBCXX_MEM_FN_TRAITS(&&, false_type, true_type)
 	__diff_type
 	_M_lookup(_Key __key, __diff_type __not_found) const
 	{
-	  auto __ukey = static_cast<std::make_unsigned_t<_Key>>(__key);
+	  auto __ukey = static_cast<make_unsigned_t<_Key>>(__key);
 	  if (__ukey >= _Len)
 	    return __not_found;
 	  return std::get<0>(_M_bad_char)[__ukey];
@@ -1046,14 +1046,14 @@  _GLIBCXX_MEM_FN_TRAITS(&&, false_type, true_type)
       const _Pred&
       _M_pred() const { return std::get<1>(_M_bad_char); }
 
-      std::tuple<_GLIBCXX_STD_C::array<_Tp, _Len>, _Pred> _M_bad_char;
+      tuple<_GLIBCXX_STD_C::array<_Tp, _Len>, _Pred> _M_bad_char;
     };
 
   template<typename _Pred>
-    struct __is_std_equal_to : std::false_type { };
+    struct __is_std_equal_to : false_type { };
 
   template<>
-    struct __is_std_equal_to<std::equal_to<void>> : std::true_type { };
+    struct __is_std_equal_to<equal_to<void>> : true_type { };
 
   // Use __boyer_moore_array_base when pattern consists of narrow characters
   // and uses std::equal_to as the predicate.
@@ -1061,14 +1061,14 @@  _GLIBCXX_MEM_FN_TRAITS(&&, false_type, true_type)
            typename _Val = typename iterator_traits<_RAIter>::value_type,
 	   typename _Diff = typename iterator_traits<_RAIter>::difference_type>
     using __boyer_moore_base_t
-      = std::conditional_t<sizeof(_Val) == 1 && is_integral<_Val>::value
-			   && __is_std_equal_to<_Pred>::value,
-			   __boyer_moore_array_base<_Diff, 256, _Pred>,
-			   __boyer_moore_map_base<_Val, _Diff, _Hash, _Pred>>;
+      = conditional_t<sizeof(_Val) == 1 && is_integral<_Val>::value
+		      && __is_std_equal_to<_Pred>::value,
+		      __boyer_moore_array_base<_Diff, 256, _Pred>,
+		      __boyer_moore_map_base<_Val, _Diff, _Hash, _Pred>>;
 
   template<typename _RAIter, typename _Hash
-	     = std::hash<typename std::iterator_traits<_RAIter>::value_type>,
-	   typename _BinaryPredicate = std::equal_to<>>
+	     = hash<typename iterator_traits<_RAIter>::value_type>,
+	   typename _BinaryPredicate = equal_to<>>
     class boyer_moore_searcher
     : __boyer_moore_base_t<_RAIter, _Hash, _BinaryPredicate>
     {
@@ -1123,8 +1123,8 @@  _GLIBCXX_MEM_FN_TRAITS(&&, false_type, true_type)
     };
 
   template<typename _RAIter, typename _Hash
-	     = std::hash<typename std::iterator_traits<_RAIter>::value_type>,
-	   typename _BinaryPredicate = std::equal_to<>>
+	     = hash<typename iterator_traits<_RAIter>::value_type>,
+	   typename _BinaryPredicate = equal_to<>>
     class boyer_moore_horspool_searcher
     : __boyer_moore_base_t<_RAIter, _Hash, _BinaryPredicate>
     {
@@ -1156,8 +1156,7 @@  _GLIBCXX_MEM_FN_TRAITS(&&, false_type, true_type)
 	      for (auto __scan = __patlen - 1;
 		   __pred(__first[__scan], _M_pat[__scan]); --__scan)
 		if (__scan == 0)
-		  return std::make_pair(__first,
-					std::next(__first, __patlen));
+		  return std::make_pair(__first, __first + __patlen);
 	      auto __shift = _M_bad_char_shift(__first[__patlen - 1]);
 	      __len -= __shift;
 	      __first += __shift;
@@ -1223,8 +1222,10 @@  _GLIBCXX_MEM_FN_TRAITS(&&, false_type, true_type)
 	      --__j;
 	    }
 	  if (__j < 0)
-	    return std::make_pair(__first + __i + 1, std::next(__first,
-							       __patlen));
+	    {
+	      const auto __match = __first + __i + 1;
+	      return std::make_pair(__match, __match + __patlen);
+	    }
 	  __i += std::max(_M_bad_char_shift(__first[__i]),
 			  _M_good_suffix[__j]);
 	}
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/searchers.cc b/libstdc++-v3/testsuite/20_util/function_objects/searchers.cc
index 9cc1455..4f7b1d4 100644
--- a/libstdc++-v3/testsuite/20_util/function_objects/searchers.cc
+++ b/libstdc++-v3/testsuite/20_util/function_objects/searchers.cc
@@ -49,7 +49,8 @@  test01()
 
   for (auto n : needles)
   {
-    auto ne = n + std::strlen(n);
+    auto nlen = std::strlen(n);
+    auto ne = n + nlen;
     default_searcher d(n, ne);
     boyer_moore_searcher bm(n, ne);
     boyer_moore_horspool_searcher bmh(n, ne);
@@ -59,10 +60,22 @@  test01()
       auto res = std::search(h, he, n, ne);
       auto d_res = d(h, he);
       VERIFY( d_res.first == res );
+      if (res == he)
+	VERIFY( d_res.second == d_res.first );
+      else
+	VERIFY( d_res.second == (d_res.first + nlen) );
       auto bm_res = bm(h, he);
       VERIFY( bm_res.first == res );
+      if (res == he)
+	VERIFY( bm_res.second == bm_res.first );
+      else
+	VERIFY( bm_res.second == (bm_res.first + nlen) );
       auto bmh_res = bmh(h, he);
       VERIFY( bmh_res.first == res );
+      if (res == he)
+	VERIFY( bmh_res.second == bmh_res.first );
+      else
+	VERIFY( bmh_res.second == (bmh_res.first + nlen) );
     }
   }
 }
@@ -82,7 +95,8 @@  test02()
 
   for (auto n : needles)
   {
-    auto ne = n + std::wcslen(n);
+    auto nlen = std::wcslen(n);
+    auto ne = n + nlen;
     default_searcher d(n, ne);
     boyer_moore_searcher bm(n, ne);
     boyer_moore_horspool_searcher bmh(n, ne);
@@ -92,10 +106,22 @@  test02()
       auto res = std::search(h, he, n, ne);
       auto d_res = d(h, he);
       VERIFY( d_res.first == res );
+      if (res == he)
+	VERIFY( d_res.second == d_res.first );
+      else
+	VERIFY( d_res.second == (d_res.first + nlen) );
       auto bm_res = bm(h, he);
       VERIFY( bm_res.first == res );
+      if (res == he)
+	VERIFY( bm_res.second == bm_res.first );
+      else
+	VERIFY( bm_res.second == (bm_res.first + nlen) );
       auto bmh_res = bmh(h, he);
       VERIFY( bmh_res.first == res );
+      if (res == he)
+	VERIFY( bmh_res.second == bmh_res.first );
+      else
+	VERIFY( bmh_res.second == (bmh_res.first + nlen) );
     }
   }
 #endif
@@ -119,7 +145,8 @@  test03()
 
   const char* needle = " foo 123 ";
   const char* haystack = "*****foo*123******";
-  const char* ne = needle + std::strlen(needle);
+  auto nlen = std::strlen(needle);
+  const char* ne = needle + nlen;
   const char* he = haystack + std::strlen(haystack);
 
   default_searcher d(needle, ne, eq);
@@ -129,10 +156,22 @@  test03()
   auto res = std::search(haystack, he, needle, ne, eq);
   auto d_res = d(haystack, he);
   VERIFY( d_res.first == res );
+  if (res == he)
+    VERIFY( d_res.second == d_res.first );
+  else
+    VERIFY( d_res.second == (d_res.first + nlen) );
   auto bm_res = bm(haystack, he);
   VERIFY( bm_res.first == res );
+  if (res == he)
+    VERIFY( bm_res.second == bm_res.first );
+  else
+    VERIFY( bm_res.second == (bm_res.first + nlen) );
   auto bmh_res = bmh(haystack, he);
   VERIFY( bmh_res.first == res );
+  if (res == he)
+    VERIFY( bmh_res.second == bmh_res.first );
+  else
+    VERIFY( bmh_res.second == (bmh_res.first + nlen) );
 }
 
 int