diff mbox

fix libstdc++/56267 - local iterator requirements

Message ID CAH6eHdQTKr_Pab4iM60Lth_ezyrbcgamYeMEssrDgXmKzxOTMQ@mail.gmail.com
State New
Headers show

Commit Message

Jonathan Wakely Jan. 21, 2014, 7:39 p.m. UTC
On 21 January 2014 10:00, Jonathan Wakely wrote:
> On 20 January 2014 21:11, François Dumont <frs.dumont@gmail.com> wrote:
>> On 01/20/2014 04:53 PM, Jonathan Wakely wrote:
>>
>> With this new design couldn't we change the conditions that are used to
>> decide when to cache the hash code. I haven't study it in detail but it
>> looks like the default constructible constraint could be removed, no ?
>
> Ah yes, I forgot about that part, I'll update the __cache_default
> definition. Thanks.

Like so. Tested x86_64-linux, committed to trunk.

2014-01-21  Jonathan Wakely  <jwakely@redhat.com>

        PR libstdc++/56267
        * include/bits/hashtable.h (__cache_default): Do not depend on
        whether the hash function is DefaultConstructible or CopyAssignable.
        (_Hashtable): Adjust static assertions.
        * doc/xml/manual/containers.xml (containers.unordered.cache): Update.
        * testsuite/23_containers/unordered_set/instantiation_neg.cc: Adjust
        dg-error line number.
        * testsuite/23_containers/unordered_set/
        not_default_constructible_hash_neg.cc: Remove.
commit 0efc402079788226108bce180c478d4c85924fd4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 21 18:20:24 2014 +0000

    	PR libstdc++/56267
    	* include/bits/hashtable.h (__cache_default): Do not depend on
    	whether the hash function is DefaultConstructible or CopyAssignable.
    	(_Hashtable): Adjust static assertions.
    	* doc/xml/manual/containers.xml (containers.unordered.cache): Update.
    	* testsuite/23_containers/unordered_set/instantiation_neg.cc: Adjust
    	dg-error line number.
    	* testsuite/23_containers/unordered_set/
    	not_default_constructible_hash_neg.cc: Remove.
diff mbox

Patch

diff --git a/libstdc++-v3/doc/xml/manual/containers.xml b/libstdc++-v3/doc/xml/manual/containers.xml
index 9791953..653033d 100644
--- a/libstdc++-v3/doc/xml/manual/containers.xml
+++ b/libstdc++-v3/doc/xml/manual/containers.xml
@@ -420,7 +420,7 @@ 
       the hash code every time it's needed can improve performance, but the
       additional memory overhead can also reduce performance, so whether an
       unordered associative container caches the hash code or not depends on
-      a number of factors. The caching policy for GCC 4.8 is described below.
+      the properties described below.
     </para>
     <para>
       The C++ standard requires that <code>erase</code> and <code>swap</code>
@@ -432,23 +432,8 @@ 
       or <code>throw()</code>.
     </para>
     <para>
-      Secondly, libstdc++ also needs the hash code in the implementation of
-      <code>local_iterator</code> and <code>const_local_iterator</code> in
-      order to know when the iterator has reached the end of the bucket.
-      This means that the local iterator types will embed a copy of the hash
-      function when possible.
-      Because the local iterator types must be DefaultConstructible and
-      CopyAssignable, if the hash function type does not model those concepts
-      then it cannot be embedded and so the hash code must be cached.
-      Note that a hash function might not be safe to use when
-      default-constructed (e.g if it a function pointer) so a hash
-      function that is contained in a local iterator won't be used until
-      the iterator is valid, so the hash function has been copied from a
-      correctly-initialized object.
-    </para>
-    <para>
-      If the hash function is non-throwing, DefaultConstructible and
-      CopyAssignable then libstdc++ doesn't need to cache the hash code for
+      If the hash function is non-throwing then libstdc++ doesn't need to
+      cache the hash code for
       correctness, but might still do so for performance if computing a
       hash code is an expensive operation, as it may be for arbitrarily
       long strings.
diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
index e427c7f..4297c5f 100644
--- a/libstdc++-v3/include/bits/hashtable.h
+++ b/libstdc++-v3/include/bits/hashtable.h
@@ -42,10 +42,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     using __cache_default
       =  __not_<__and_<// Do not cache for fast hasher.
 		       __is_fast_hash<_Hash>,
-		       // Mandatory to make local_iterator default
-		       // constructible and assignable.
-		       is_default_constructible<_Hash>,
-		       is_copy_assignable<_Hash>,
 		       // Mandatory to have erase not throwing.
 		       __detail::__is_noexcept_hash<_Tp, _Hash>>>;
 
@@ -282,22 +278,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		    "Functor used to map hash code to bucket index"
 		    " must be default constructible");
 
-      // When hash codes are not cached local iterator inherits from
-      // __hash_code_base above to compute node bucket index so it has to be
-      // default constructible.
-      static_assert(__if_hash_not_cached<
-		    is_default_constructible<__hash_code_base_access>>::value,
-		    "Cache the hash code or make functors involved in hash code"
-		    " and bucket index computation default constructible");
-
-      // When hash codes are not cached local iterator inherits from
-      // __hash_code_base above to compute node bucket index so it has to be
-      // assignable.
-      static_assert(__if_hash_not_cached<
-		      is_copy_assignable<__hash_code_base>>::value,
-		    "Cache the hash code or make functors involved in hash code"
-		    " and bucket index computation copy assignable");
-
       template<typename _Keya, typename _Valuea, typename _Alloca,
 	       typename _ExtractKeya, typename _Equala,
 	       typename _H1a, typename _H2a, typename _Hasha,
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/instantiation_neg.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/instantiation_neg.cc
index 83bc9d8..30784f8 100644
--- a/libstdc++-v3/testsuite/23_containers/unordered_set/instantiation_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/unordered_set/instantiation_neg.cc
@@ -19,7 +19,7 @@ 
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// { dg-error "with noexcept" "" { target *-*-* } 270 }
+// { dg-error "with noexcept" "" { target *-*-* } 266 }
 
 #include <unordered_set>
 
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/not_default_constructible_hash_neg.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/not_default_constructible_hash_neg.cc
deleted file mode 100644
index 2365556..0000000
--- a/libstdc++-v3/testsuite/23_containers/unordered_set/not_default_constructible_hash_neg.cc
+++ /dev/null
@@ -1,51 +0,0 @@ 
-// { dg-do compile }
-// { dg-options "-std=c++11" }
-// { dg-require-normal-mode "" }
-
-// Copyright (C) 2013-2014 Free Software Foundation, Inc.
-//
-// This file is part of the GNU ISO C++ Library.  This library is free
-// software; you can redistribute it and/or modify it under the
-// terms of the GNU General Public License as published by the
-// Free Software Foundation; either version 3, or (at your option)
-// any later version.
-
-// This library is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-
-// You should have received a copy of the GNU General Public License along
-// with this library; see the file COPYING3.  If not see
-// <http://www.gnu.org/licenses/>.
-
-// { dg-error "default constructible" "" { target *-*-* } 288 }
-
-#include <unordered_set>
-
-namespace
-{
-  struct hash
-  {
-    hash(std::size_t seed)
-      : _M_seed(seed)
-    { }
-
-    std::size_t operator() (int val) const noexcept
-    { return val ^ _M_seed; }
-
-  private:
-    std::size_t _M_seed;
-  };
-}
-
-void
-test01()
-{
-  using traits = std::__detail::_Hashtable_traits<false, true, true>;
-  using hashtable = std::__uset_hashtable<int, hash,
-					  std::equal_to<int>,
-					  std::allocator<int>, traits>;
-
-  hashtable ht(10, hash(1));
-}