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 <jwak...@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); }
   };
 
   /**

Reply via email to