Remove redundant accessors in hash tables

Message ID 20190515153716.GX2599@redhat.com
State New
Headers show
Series
  • Remove redundant accessors in hash tables
Related show

Commit Message

Jonathan Wakely May 15, 2019, 3:37 p.m.
François,
I noticed that _Hash_code_base and _Hashtable_base have a number of
member functions which are overloaded for const and non-const:

    const _Equal&
    _M_eq() const { return _EqualEBO::_S_cget(*this); }

    _Equal&
    _M_eq() { return _EqualEBO::_S_get(*this); }

The non-const ones seem to be unnecessary. They're used in the _M_swap
member functions, but all other uses could (and probably should) call
the const overload to get a const reference to the function object.

If we make the _M_swap members use the EBO accessors directly then we
can get rid of the non-const accessors. That makes overload resolution
simpler for the compiler (as there's only one function to choose from)
and should result in slightly smaller code when inlining is not
enabled.

Do you see any problem with this patch?

Comments

François Dumont May 16, 2019, 5:47 a.m. | #1
On 5/15/19 5:37 PM, Jonathan Wakely wrote:
> François,

> I noticed that _Hash_code_base and _Hashtable_base have a number of

> member functions which are overloaded for const and non-const:

>

>    const _Equal&

>    _M_eq() const { return _EqualEBO::_S_cget(*this); }

>

>    _Equal&

>    _M_eq() { return _EqualEBO::_S_get(*this); }

>

> The non-const ones seem to be unnecessary. They're used in the _M_swap

> member functions, but all other uses could (and probably should) call

> the const overload to get a const reference to the function object.

>

> If we make the _M_swap members use the EBO accessors directly then we

> can get rid of the non-const accessors. That makes overload resolution

> simpler for the compiler (as there's only one function to choose from)

> and should result in slightly smaller code when inlining is not

> enabled.

>

> Do you see any problem with this patch?

>

>

I think it is more a Pavlov behavior, always providing const and 
non-const no matter what.

No problem to simplify this.
Jonathan Wakely May 16, 2019, 10:05 a.m. | #2
On 16/05/19 07:47 +0200, François Dumont wrote:
>On 5/15/19 5:37 PM, Jonathan Wakely wrote:

>>François,

>>I noticed that _Hash_code_base and _Hashtable_base have a number of

>>member functions which are overloaded for const and non-const:

>>

>>   const _Equal&

>>   _M_eq() const { return _EqualEBO::_S_cget(*this); }

>>

>>   _Equal&

>>   _M_eq() { return _EqualEBO::_S_get(*this); }

>>

>>The non-const ones seem to be unnecessary. They're used in the _M_swap

>>member functions, but all other uses could (and probably should) call

>>the const overload to get a const reference to the function object.

>>

>>If we make the _M_swap members use the EBO accessors directly then we

>>can get rid of the non-const accessors. That makes overload resolution

>>simpler for the compiler (as there's only one function to choose from)

>>and should result in slightly smaller code when inlining is not

>>enabled.

>>

>>Do you see any problem with this patch?

>>

>>

>I think it is more a Pavlov behavior, always providing const and 

>non-const no matter what.

>

>No problem to simplify this.


OK, tested powerpc64le-linux, committed to trunk.
commit 4b1784930a4d819d950f776de0fa238ccc11a067
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu May 16 10:11:52 2019 +0100

    Remove unnecessary non-const accessors in hash table bases
    
    The const accessors are OK (and arguably more correct) for most callers
    to use. The _M_swap functions that use the non-const overloads can just
    directly use the _S_get members of the EBO helpers.
    
            * include/bits/hashtable_policy.h (_Hash_code_base::_M_swap): Use
            _S_get accessors for members in EBO helpers.
            (_Hash_code_base::_M_extract(), _Hash_code_base::_M_ranged_hash())
            (_Hash_code_base::_M_h1(), _Hash_code_base::_M_h2()): Remove non-const
            overloads.
            (_Hashtable_base::_M_swap): Use _S_get accessors for members in EBO
            helpers.
            (_Hashtable_base::_M_eq()): Remove non-const overload.

diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index c7f466cd686..b417a7d442c 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -1229,21 +1229,16 @@ namespace __detail
       void
       _M_swap(_Hash_code_base& __x)
       {
-	std::swap(_M_extract(), __x._M_extract());
-	std::swap(_M_ranged_hash(), __x._M_ranged_hash());
+	std::swap(__ebo_extract_key::_S_get(*this),
+		  __ebo_extract_key::_S_get(__x));
+	std::swap(__ebo_hash::_S_get(*this), __ebo_hash::_S_get(__x));
       }
 
       const _ExtractKey&
       _M_extract() const { return __ebo_extract_key::_S_cget(*this); }
 
-      _ExtractKey&
-      _M_extract() { return __ebo_extract_key::_S_get(*this); }
-
       const _Hash&
       _M_ranged_hash() const { return __ebo_hash::_S_cget(*this); }
-
-      _Hash&
-      _M_ranged_hash() { return __ebo_hash::_S_get(*this); }
     };
 
   // No specialization for ranged hash function while caching hash codes.
@@ -1322,28 +1317,20 @@ namespace __detail
       void
       _M_swap(_Hash_code_base& __x)
       {
-	std::swap(_M_extract(), __x._M_extract());
-	std::swap(_M_h1(), __x._M_h1());
-	std::swap(_M_h2(), __x._M_h2());
+	std::swap(__ebo_extract_key::_S_get(*this),
+		  __ebo_extract_key::_S_get(__x));
+	std::swap(__ebo_h1::_S_get(*this), __ebo_h1::_S_get(__x));
+	std::swap(__ebo_h2::_S_get(*this), __ebo_h2::_S_get(__x));
       }
 
       const _ExtractKey&
       _M_extract() const { return __ebo_extract_key::_S_cget(*this); }
 
-      _ExtractKey&
-      _M_extract() { return __ebo_extract_key::_S_get(*this); }
-
       const _H1&
       _M_h1() const { return __ebo_h1::_S_cget(*this); }
 
-      _H1&
-      _M_h1() { return __ebo_h1::_S_get(*this); }
-
       const _H2&
       _M_h2() const { return __ebo_h2::_S_cget(*this); }
-
-      _H2&
-      _M_h2() { return __ebo_h2::_S_get(*this); }
     };
 
   /// Specialization: hash function and range-hashing function,
@@ -1410,28 +1397,20 @@ namespace __detail
       void
       _M_swap(_Hash_code_base& __x)
       {
-	std::swap(_M_extract(), __x._M_extract());
-	std::swap(_M_h1(), __x._M_h1());
-	std::swap(_M_h2(), __x._M_h2());
+	std::swap(__ebo_extract_key::_S_get(*this),
+		  __ebo_extract_key::_S_get(__x));
+	std::swap(__ebo_h1::_S_get(*this), __ebo_h1::_S_get(__x));
+	std::swap(__ebo_h2::_S_get(*this), __ebo_h2::_S_get(__x));
       }
 
       const _ExtractKey&
       _M_extract() const { return __ebo_extract_key::_S_cget(*this); }
 
-      _ExtractKey&
-      _M_extract() { return __ebo_extract_key::_S_get(*this); }
-
       const _H1&
       _M_h1() const { return __ebo_h1::_S_cget(*this); }
 
-      _H1&
-      _M_h1() { return __ebo_h1::_S_get(*this); }
-
       const _H2&
       _M_h2() const { return __ebo_h2::_S_cget(*this); }
-
-      _H2&
-      _M_h2() { return __ebo_h2::_S_get(*this); }
     };
 
   /**
@@ -1840,14 +1819,11 @@ namespace __detail
     _M_swap(_Hashtable_base& __x)
     {
       __hash_code_base::_M_swap(__x);
-      std::swap(_M_eq(), __x._M_eq());
+      std::swap(_EqualEBO::_S_get(*this), _EqualEBO::_S_get(__x));
     }
 
     const _Equal&
     _M_eq() const { return _EqualEBO::_S_cget(*this); }
-
-    _Equal&
-    _M_eq() { return _EqualEBO::_S_get(*this); }
   };
 
   /**
Jonathan Wakely May 16, 2019, 12:30 p.m. | #3
On 16/05/19 11:05 +0100, Jonathan Wakely wrote:
>On 16/05/19 07:47 +0200, François Dumont wrote:

>>On 5/15/19 5:37 PM, Jonathan Wakely wrote:

>>>François,

>>>I noticed that _Hash_code_base and _Hashtable_base have a number of

>>>member functions which are overloaded for const and non-const:

>>>

>>>   const _Equal&

>>>   _M_eq() const { return _EqualEBO::_S_cget(*this); }

>>>

>>>   _Equal&

>>>   _M_eq() { return _EqualEBO::_S_get(*this); }

>>>

>>>The non-const ones seem to be unnecessary. They're used in the _M_swap

>>>member functions, but all other uses could (and probably should) call

>>>the const overload to get a const reference to the function object.

>>>

>>>If we make the _M_swap members use the EBO accessors directly then we

>>>can get rid of the non-const accessors. That makes overload resolution

>>>simpler for the compiler (as there's only one function to choose from)

>>>and should result in slightly smaller code when inlining is not

>>>enabled.

>>>

>>>Do you see any problem with this patch?

>>>

>>>

>>I think it is more a Pavlov behavior, always providing const and 

>>non-const no matter what.

>>

>>No problem to simplify this.

>

>OK, tested powerpc64le-linux, committed to trunk.


I don't see a need for the _Hashtable_ebo_helper member functions to
be static. They return a member of *this, but are static so *this has
to be passed as a parameter. We could just make them non-static.

It seems to have always been that way since the first version of the
patch that added the helpers:
https://gcc.gnu.org/ml/libstdc++/2011-12/msg00139.html

This patch passes all tests. I plan to commit this to trunk too.
commit 73b18aaa3a42f446b3bf295ef5bcd2e3af45004f
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu May 16 13:01:34 2019 +0100

    Change EBO accessors from static to non-static member functions
    
            * include/bits/hashtable_policy.h (_Hashtable_ebo_helper::_S_get):
            Replace with _M_get non-static member function.
            (_Hashtable_ebo_helper::_S_cget): Replace with _M_cget non-static
            member function.
            (_Hash_code_base, _Local_iterator_base, _Hashtable_base):
            (_Hashtable_alloc): Adjust to use non-static members of EBO helper.

diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index b417a7d442c..f7db7628c69 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -1112,13 +1112,8 @@ namespace __detail
 	  : _Tp(std::forward<_OtherTp>(__tp))
 	{ }
 
-      static const _Tp&
-      _S_cget(const _Hashtable_ebo_helper& __eboh)
-      { return static_cast<const _Tp&>(__eboh); }
-
-      static _Tp&
-      _S_get(_Hashtable_ebo_helper& __eboh)
-      { return static_cast<_Tp&>(__eboh); }
+      const _Tp& _M_cget() const { return static_cast<const _Tp&>(*this); }
+      _Tp& _M_get() { return static_cast<_Tp&>(*this); }
     };
 
   /// Specialization not using EBO.
@@ -1132,13 +1127,8 @@ namespace __detail
 	  : _M_tp(std::forward<_OtherTp>(__tp))
 	{ }
 
-      static const _Tp&
-      _S_cget(const _Hashtable_ebo_helper& __eboh)
-      { return __eboh._M_tp; }
-
-      static _Tp&
-      _S_get(_Hashtable_ebo_helper& __eboh)
-      { return __eboh._M_tp; }
+      const _Tp& _M_cget() const { return _M_tp; }
+      _Tp& _M_get() { return _M_tp; }
 
     private:
       _Tp _M_tp;
@@ -1229,16 +1219,16 @@ namespace __detail
       void
       _M_swap(_Hash_code_base& __x)
       {
-	std::swap(__ebo_extract_key::_S_get(*this),
-		  __ebo_extract_key::_S_get(__x));
-	std::swap(__ebo_hash::_S_get(*this), __ebo_hash::_S_get(__x));
+	std::swap(__ebo_extract_key::_M_get(),
+		  __x.__ebo_extract_key::_M_get());
+	std::swap(__ebo_hash::_M_get(), __x.__ebo_hash::_M_get());
       }
 
       const _ExtractKey&
-      _M_extract() const { return __ebo_extract_key::_S_cget(*this); }
+      _M_extract() const { return __ebo_extract_key::_M_cget(); }
 
       const _Hash&
-      _M_ranged_hash() const { return __ebo_hash::_S_cget(*this); }
+      _M_ranged_hash() const { return __ebo_hash::_M_cget(); }
     };
 
   // No specialization for ranged hash function while caching hash codes.
@@ -1317,20 +1307,20 @@ namespace __detail
       void
       _M_swap(_Hash_code_base& __x)
       {
-	std::swap(__ebo_extract_key::_S_get(*this),
-		  __ebo_extract_key::_S_get(__x));
-	std::swap(__ebo_h1::_S_get(*this), __ebo_h1::_S_get(__x));
-	std::swap(__ebo_h2::_S_get(*this), __ebo_h2::_S_get(__x));
+	std::swap(__ebo_extract_key::_M_get(),
+		  __x.__ebo_extract_key::_M_get());
+	std::swap(__ebo_h1::_M_get(), __x.__ebo_h1::_M_get());
+	std::swap(__ebo_h2::_M_get(), __x.__ebo_h2::_M_get());
       }
 
       const _ExtractKey&
-      _M_extract() const { return __ebo_extract_key::_S_cget(*this); }
+      _M_extract() const { return __ebo_extract_key::_M_cget(); }
 
       const _H1&
-      _M_h1() const { return __ebo_h1::_S_cget(*this); }
+      _M_h1() const { return __ebo_h1::_M_cget(); }
 
       const _H2&
-      _M_h2() const { return __ebo_h2::_S_cget(*this); }
+      _M_h2() const { return __ebo_h2::_M_cget(); }
     };
 
   /// Specialization: hash function and range-hashing function,
@@ -1397,20 +1387,20 @@ namespace __detail
       void
       _M_swap(_Hash_code_base& __x)
       {
-	std::swap(__ebo_extract_key::_S_get(*this),
-		  __ebo_extract_key::_S_get(__x));
-	std::swap(__ebo_h1::_S_get(*this), __ebo_h1::_S_get(__x));
-	std::swap(__ebo_h2::_S_get(*this), __ebo_h2::_S_get(__x));
+	std::swap(__ebo_extract_key::_M_get(),
+		  __x.__ebo_extract_key::_M_get());
+	std::swap(__ebo_h1::_M_get(), __x.__ebo_h1::_M_get());
+	std::swap(__ebo_h2::_M_get(), __x.__ebo_h2::_M_get());
       }
 
       const _ExtractKey&
-      _M_extract() const { return __ebo_extract_key::_S_cget(*this); }
+      _M_extract() const { return __ebo_extract_key::_M_cget(); }
 
       const _H1&
-      _M_h1() const { return __ebo_h1::_S_cget(*this); }
+      _M_h1() const { return __ebo_h1::_M_cget(); }
 
       const _H2&
-      _M_h2() const { return __ebo_h2::_S_cget(*this); }
+      _M_h2() const { return __ebo_h2::_M_cget(); }
     };
 
   /**
@@ -1471,7 +1461,7 @@ namespace __detail
 	if (_M_cur)
 	  {
 	    std::size_t __bkt
-	      = __base_type::_S_get(*this)(_M_cur->_M_hash_code,
+	      = __base_type::_M_get()(_M_cur->_M_hash_code,
 					   _M_bucket_count);
 	    if (__bkt != _M_bucket)
 	      _M_cur = nullptr;
@@ -1819,11 +1809,11 @@ namespace __detail
     _M_swap(_Hashtable_base& __x)
     {
       __hash_code_base::_M_swap(__x);
-      std::swap(_EqualEBO::_S_get(*this), _EqualEBO::_S_get(__x));
+      std::swap(_EqualEBO::_M_get(), __x._EqualEBO::_M_get());
     }
 
     const _Equal&
-    _M_eq() const { return _EqualEBO::_S_cget(*this); }
+    _M_eq() const { return _EqualEBO::_M_cget(); }
   };
 
   /**
@@ -2021,11 +2011,11 @@ namespace __detail
 
       __node_alloc_type&
       _M_node_allocator()
-      { return __ebo_node_alloc::_S_get(*this); }
+      { return __ebo_node_alloc::_M_get(); }
 
       const __node_alloc_type&
       _M_node_allocator() const
-      { return __ebo_node_alloc::_S_cget(*this); }
+      { return __ebo_node_alloc::_M_cget(); }
 
       template<typename... _Args>
 	__node_type*
Jonathan Wakely May 16, 2019, 1:24 p.m. | #4
On 16/05/19 13:30 +0100, Jonathan Wakely wrote:
>On 16/05/19 11:05 +0100, Jonathan Wakely wrote:

>>On 16/05/19 07:47 +0200, François Dumont wrote:

>>>On 5/15/19 5:37 PM, Jonathan Wakely wrote:

>>>>François,

>>>>I noticed that _Hash_code_base and _Hashtable_base have a number of

>>>>member functions which are overloaded for const and non-const:

>>>>

>>>>   const _Equal&

>>>>   _M_eq() const { return _EqualEBO::_S_cget(*this); }

>>>>

>>>>   _Equal&

>>>>   _M_eq() { return _EqualEBO::_S_get(*this); }

>>>>

>>>>The non-const ones seem to be unnecessary. They're used in the _M_swap

>>>>member functions, but all other uses could (and probably should) call

>>>>the const overload to get a const reference to the function object.

>>>>

>>>>If we make the _M_swap members use the EBO accessors directly then we

>>>>can get rid of the non-const accessors. That makes overload resolution

>>>>simpler for the compiler (as there's only one function to choose from)

>>>>and should result in slightly smaller code when inlining is not

>>>>enabled.

>>>>

>>>>Do you see any problem with this patch?

>>>>

>>>>

>>>I think it is more a Pavlov behavior, always providing const and 

>>>non-const no matter what.

>>>

>>>No problem to simplify this.

>>

>>OK, tested powerpc64le-linux, committed to trunk.

>

>I don't see a need for the _Hashtable_ebo_helper member functions to

>be static. They return a member of *this, but are static so *this has

>to be passed as a parameter. We could just make them non-static.

>

>It seems to have always been that way since the first version of the

>patch that added the helpers:

>https://gcc.gnu.org/ml/libstdc++/2011-12/msg00139.html

>

>This patch passes all tests. I plan to commit this to trunk too.


And another one (which I've actually had sitting in a local branch for
a few years!)
commit 4cf9940963e326d09080177d7404f5aaedae67df
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Dec 5 10:42:54 2014 +0000

    Replace _Equal_helper with simpler class template
    
    By defining the new helper inside _Hashtable_base it doesn't need all
    the template parameters to be provided, and by making it only
    responsible for checking a possibly-cached hash code it only has to do
    one thing.  The caller can use the equality predicate itself instead of
    duplicating that in the helper template.
    
            * include/bits/hashtable_policy.h (_Equal_helper): Remove.
            (_Hashtable_base::_Equal_hash_code): Define new class template.
            (_Hashtable_base::_M_equals): Use _Equal_hash_code instead of
            _Equal_helper.

diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index f7db7628c69..86589e9a2d6 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -1403,38 +1403,6 @@ namespace __detail
       _M_h2() const { return __ebo_h2::_M_cget(); }
     };
 
-  /**
-   *  Primary class template _Equal_helper.
-   *
-   */
-  template <typename _Key, typename _Value, typename _ExtractKey,
-	    typename _Equal, typename _HashCodeType,
-	    bool __cache_hash_code>
-  struct _Equal_helper;
-
-  /// Specialization.
-  template<typename _Key, typename _Value, typename _ExtractKey,
-	   typename _Equal, typename _HashCodeType>
-  struct _Equal_helper<_Key, _Value, _ExtractKey, _Equal, _HashCodeType, true>
-  {
-    static bool
-    _S_equals(const _Equal& __eq, const _ExtractKey& __extract,
-	      const _Key& __k, _HashCodeType __c, _Hash_node<_Value, true>* __n)
-    { return __c == __n->_M_hash_code && __eq(__k, __extract(__n->_M_v())); }
-  };
-
-  /// Specialization.
-  template<typename _Key, typename _Value, typename _ExtractKey,
-	   typename _Equal, typename _HashCodeType>
-  struct _Equal_helper<_Key, _Value, _ExtractKey, _Equal, _HashCodeType, false>
-  {
-    static bool
-    _S_equals(const _Equal& __eq, const _ExtractKey& __extract,
-	      const _Key& __k, _HashCodeType, _Hash_node<_Value, false>* __n)
-    { return __eq(__k, __extract(__n->_M_v())); }
-  };
-
-
   /// Partial specialization used when nodes contain a cached hash code.
   template<typename _Key, typename _Value, typename _ExtractKey,
 	   typename _H1, typename _H2, typename _Hash>
@@ -1788,8 +1756,22 @@ namespace __detail
 						     iterator>::type;
   private:
     using _EqualEBO = _Hashtable_ebo_helper<0, _Equal>;
-    using _EqualHelper =  _Equal_helper<_Key, _Value, _ExtractKey, _Equal,
-					__hash_code, __hash_cached::value>;
+
+    template<typename _NodeT>
+      struct _Equal_hash_code
+      {
+       static bool
+       _S_equals(__hash_code, const _NodeT&)
+       { return true; }
+      };
+
+    template<typename _Ptr2>
+      struct _Equal_hash_code<_Hash_node<_Ptr2, true>>
+      {
+       static bool
+       _S_equals(__hash_code __c, const _Hash_node<_Ptr2, true>& __n)
+       { return __c == __n._M_hash_code; }
+      };
 
   protected:
     _Hashtable_base() = default;
@@ -1801,8 +1783,8 @@ namespace __detail
     bool
     _M_equals(const _Key& __k, __hash_code __c, __node_type* __n) const
     {
-      return _EqualHelper::_S_equals(_M_eq(), this->_M_extract(),
-				     __k, __c, __n);
+      return _Equal_hash_code<__node_type>::_S_equals(__c, *__n)
+	&& _M_eq()(__k, this->_M_extract()(__n->_M_v()));
     }
 
     void

Patch

diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index 9505fb8d261..5ff90c952ba 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -1229,21 +1229,16 @@  namespace __detail
       void
       _M_swap(_Hash_code_base& __x)
       {
-	std::swap(_M_extract(), __x._M_extract());
-	std::swap(_M_ranged_hash(), __x._M_ranged_hash());
+	std::swap(__ebo_extract_key::_S_get(*this),
+		  __ebo_extract_key::_S_get(__x));
+	std::swap(__ebo_hash::_S_get(*this), __ebo_hash::_S_get(__x));
       }
 
       const _ExtractKey&
       _M_extract() const { return __ebo_extract_key::_S_cget(*this); }
 
-      _ExtractKey&
-      _M_extract() { return __ebo_extract_key::_S_get(*this); }
-
       const _Hash&
       _M_ranged_hash() const { return __ebo_hash::_S_cget(*this); }
-
-      _Hash&
-      _M_ranged_hash() { return __ebo_hash::_S_get(*this); }
     };
 
   // No specialization for ranged hash function while caching hash codes.
@@ -1326,28 +1321,20 @@  namespace __detail
       void
       _M_swap(_Hash_code_base& __x)
       {
-	std::swap(_M_extract(), __x._M_extract());
-	std::swap(_M_h1(), __x._M_h1());
-	std::swap(_M_h2(), __x._M_h2());
+	std::swap(__ebo_extract_key::_S_get(*this),
+		  __ebo_extract_key::_S_get(__x));
+	std::swap(__ebo_h1::_S_get(*this), __ebo_h1::_S_get(__x));
+	std::swap(__ebo_h2::_S_get(*this), __ebo_h2::_S_get(__x));
       }
 
       const _ExtractKey&
       _M_extract() const { return __ebo_extract_key::_S_cget(*this); }
 
-      _ExtractKey&
-      _M_extract() { return __ebo_extract_key::_S_get(*this); }
-
       const _H1&
       _M_h1() const { return __ebo_h1::_S_cget(*this); }
 
-      _H1&
-      _M_h1() { return __ebo_h1::_S_get(*this); }
-
       const _H2&
       _M_h2() const { return __ebo_h2::_S_cget(*this); }
-
-      _H2&
-      _M_h2() { return __ebo_h2::_S_get(*this); }
     };
 
   /// Specialization: hash function and range-hashing function,
@@ -1418,28 +1405,20 @@  namespace __detail
       void
       _M_swap(_Hash_code_base& __x)
       {
-	std::swap(_M_extract(), __x._M_extract());
-	std::swap(_M_h1(), __x._M_h1());
-	std::swap(_M_h2(), __x._M_h2());
+	std::swap(__ebo_extract_key::_S_get(*this),
+		  __ebo_extract_key::_S_get(__x));
+	std::swap(__ebo_h1::_S_get(*this), __ebo_h1::_S_get(__x));
+	std::swap(__ebo_h2::_S_get(*this), __ebo_h2::_S_get(__x));
       }
 
       const _ExtractKey&
       _M_extract() const { return __ebo_extract_key::_S_cget(*this); }
 
-      _ExtractKey&
-      _M_extract() { return __ebo_extract_key::_S_get(*this); }
-
       const _H1&
       _M_h1() const { return __ebo_h1::_S_cget(*this); }
 
-      _H1&
-      _M_h1() { return __ebo_h1::_S_get(*this); }
-
       const _H2&
       _M_h2() const { return __ebo_h2::_S_cget(*this); }
-
-      _H2&
-      _M_h2() { return __ebo_h2::_S_get(*this); }
     };
 
   /**
@@ -1851,14 +1830,11 @@  namespace __detail
     _M_swap(_Hashtable_base& __x)
     {
       __hash_code_base::_M_swap(__x);
-      std::swap(_M_eq(), __x._M_eq());
+      std::swap(_EqualEBO::_S_get(*this), _EqualEBO::_S_get(__x));
     }
 
     const _Equal&
     _M_eq() const { return _EqualEBO::_S_cget(*this); }
-
-    _Equal&
-    _M_eq() { return _EqualEBO::_S_get(*this); }
   };
 
   /**