diff mbox series

libstdc++: Define operator== for hash table iterators [PR115939]

Message ID 20240821202757.570009-1-jwakely@redhat.com
State New
Headers show
Series libstdc++: Define operator== for hash table iterators [PR115939] | expand

Commit Message

Jonathan Wakely Aug. 21, 2024, 8:27 p.m. UTC 
Tested x86_64-linux. I plan to push this soon.

-- >8 --

Currently iterators for unordered containers do not directly define
operator== and operator!= overloads. Instead they rely on the base class
defining them, which is done so that iterator and const_iterator
comparisons work using the same overloads.

However this means a derived-to-base conversion is needed to call those
operators, and PR libstdc++/115939 shows that this can be ambiguous (for
-pedantic) when another overloaded operator could be used after an
implicit conversion.

This change defines operator== and operator!= directly for
_Node_iterator and _Node_const_iterator so that no derived-to-base
conversions are needed. The new overloads just forward to the base class
ones, so the implementation is still shared and doesn't need to be
duplicated.

libstdc++-v3/ChangeLog:

	PR libstdc++/115939
	* include/bits/hashtable_policy.h (_Node_iterator): Add
	operator== and operator!=.
	(_Node_const_iterator): Likewise.
	* testsuite/23_containers/unordered_map/115939.cc: New test.
---
 libstdc++-v3/include/bits/hashtable_policy.h  | 80 ++++++++++++++++++-
 .../23_containers/unordered_map/115939.cc     | 29 +++++++
 2 files changed, 107 insertions(+), 2 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/23_containers/unordered_map/115939.cc
diff mbox series

Patch

diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index 26def24f24e..1b9d9aae1a7 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -464,6 +464,23 @@  namespace __detail
 	this->_M_incr();
 	return __tmp;
       }
+
+#if __cpp_impl_three_way_comparison >= 201907L
+      friend bool
+      operator==(const _Node_iterator&, const _Node_iterator&) = default;
+#else
+      friend bool
+      operator==(const _Node_iterator& __x, const _Node_iterator& __y) noexcept
+      {
+	const __base_type& __bx = __x;
+	const __base_type& __by = __y;
+	return __bx == __by;
+      }
+
+      friend bool
+      operator!=(const _Node_iterator& __x, const _Node_iterator& __y) noexcept
+      { return !(__x == __y); }
+#endif
     };
 
   /// Node const_iterators, used to iterate through all the hashtable.
@@ -475,6 +492,10 @@  namespace __detail
       using __base_type = _Node_iterator_base<_Value, __cache>;
       using __node_type = typename __base_type::__node_type;
 
+      // The corresponding non-const iterator.
+      using __iterator
+	= _Node_iterator<_Value, __constant_iterators, __cache>;
+
     public:
       typedef _Value					value_type;
       typedef std::ptrdiff_t				difference_type;
@@ -489,8 +510,7 @@  namespace __detail
       _Node_const_iterator(__node_type* __p) noexcept
       : __base_type(__p) { }
 
-      _Node_const_iterator(const _Node_iterator<_Value, __constant_iterators,
-			   __cache>& __x) noexcept
+      _Node_const_iterator(const __iterator& __x) noexcept
       : __base_type(__x._M_cur) { }
 
       reference
@@ -515,6 +535,62 @@  namespace __detail
 	this->_M_incr();
 	return __tmp;
       }
+
+#if __cpp_impl_three_way_comparison >= 201907L
+      friend bool
+      operator==(const _Node_const_iterator&,
+		 const _Node_const_iterator&) = default;
+
+      friend bool
+      operator==(const _Node_const_iterator& __x, const __iterator& __y)
+      {
+	const __base_type& __bx = __x;
+	const __base_type& __by = __y;
+	return __bx == __by;
+      }
+#else
+      friend bool
+      operator==(const _Node_const_iterator& __x,
+		 const _Node_const_iterator& __y) noexcept
+      {
+	const __base_type& __bx = __x;
+	const __base_type& __by = __y;
+	return __bx == __by;
+      }
+
+      friend bool
+      operator!=(const _Node_const_iterator& __x,
+		 const _Node_const_iterator& __y) noexcept
+      { return !(__x == __y); }
+
+      friend bool
+      operator==(const _Node_const_iterator& __x,
+		 const __iterator& __y) noexcept
+      {
+	const __base_type& __bx = __x;
+	const __base_type& __by = __y;
+	return __bx == __by;
+      }
+
+      friend bool
+      operator!=(const _Node_const_iterator& __x,
+		 const __iterator& __y) noexcept
+      { return !(__x == __y); }
+
+      friend bool
+      operator==(const __iterator& __x,
+		 const _Node_const_iterator& __y) noexcept
+      {
+	const __base_type& __bx = __x;
+	const __base_type& __by = __y;
+	return __bx == __by;
+      }
+
+      friend bool
+      operator!=(const __iterator& __x,
+		 const _Node_const_iterator& __y) noexcept
+      { return !(__x == __y); }
+#endif
     };
 
   // Many of class template _Hashtable's template parameters are policy
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/115939.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/115939.cc
new file mode 100644
index 00000000000..3a68acaff77
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/unordered_map/115939.cc
@@ -0,0 +1,29 @@ 
+// { dg-options "-pedantic" }
+// { dg-do compile { target c++11 } }
+
+// Bug 115939 - Cannot unambiguously compare iterators in std::unordered_map
+
+#include <unordered_map>
+
+struct any_conv
+{
+  template<typename T>
+    any_conv(T&&) { }
+
+  template<typename T>
+    friend bool operator==(const any_conv&, const T&) { return true; }
+
+  template<typename T>
+    friend bool operator!=(const any_conv&, const T&) { return false; }
+};
+
+std::unordered_map<int, any_conv>::iterator i{};
+std::unordered_map<int, any_conv>::const_iterator j{};
+bool b1 = i == i; // { dg-bogus "ambiguous" }
+bool b2 = j == j; // { dg-bogus "ambiguous" }
+bool b3 = i == j; // { dg-bogus "ambiguous" }
+bool b4 = j == i; // { dg-bogus "ambiguous" }
+bool b5 = i != i; // { dg-bogus "ambiguous" }
+bool b6 = j != j; // { dg-bogus "ambiguous" }
+bool b7 = i != j; // { dg-bogus "ambiguous" }
+bool b8 = j != i; // { dg-bogus "ambiguous" }