diff mbox

PR 59712 patch

Message ID 52CF1ADB.5070901@gmail.com
State New
Headers show

Commit Message

François Dumont Jan. 9, 2014, 9:55 p.m. UTC 
Hi

     Here is a patch for this small problem with clang. It is not a 
blocking issue for the 4.9 release but at the same time it is a rather 
safe fix so just tell me if I can commit it.

     All unordered_* tests run under Linux x86_64. I haven't clang 
installed at the moment so a clang feedback would be appreciated.

2014-01-10  François Dumont  <fdumont@gcc.gnu.org>

     * include/bits/hashtable_policy.h: Fix some long lines.
     * include/bits/hashtable.h (__hash_code_base_access): Define and
     use it to check its _M_bucket_index noexcept qualification. Use
     also in place of...
     (__access_protected_ctor): ...this.
     * testsuite/23_containers/unordered_set/instantiation_neg.cc:
     Adapt line number.
     * testsuite/23_containers/unordered_set/
     not_default_constructible_hash_neg.cc: Likewise.

François

Comments

Jonathan Wakely Jan. 9, 2014, 10:49 p.m. UTC | #1 
On 9 January 2014 21:55, François Dumont wrote:
>
>     All unordered_* tests run under Linux x86_64.

Please make sure you run the entire testsuite, not just the parts that
seem relevant.
François Dumont Jan. 11, 2014, 3:59 p.m. UTC | #2 
On 01/09/2014 11:49 PM, Jonathan Wakely wrote:
> On 9 January 2014 21:55, François Dumont wrote:
>>      All unordered_* tests run under Linux x86_64.
> Please make sure you run the entire testsuite, not just the parts that
> seem relevant.
>
Done and no failure, ok to commit ?

François
Jonathan Wakely Jan. 14, 2014, 2:44 p.m. UTC | #3 
On 9 January 2014 21:55, François Dumont wrote:
> Hi
>
>     Here is a patch for this small problem with clang. It is not a blocking
> issue for the 4.9 release but at the same time it is a rather safe fix so
> just tell me if I can commit it.
>
>     All unordered_* tests run under Linux x86_64. I haven't clang installed
> at the moment so a clang feedback would be appreciated.

I'm going to test this with Clang to be sure it actually fixes the bug
before we change anything, but it might have to wait for the weekend.

N.B. please change the comment at the top of the patch to say "pool of
nodes" instead of "pool of node"
Jonathan Wakely Jan. 15, 2014, 11:37 a.m. UTC | #4 
On 14 January 2014 14:44, Jonathan Wakely wrote:
> On 9 January 2014 21:55, François Dumont wrote:
>> Hi
>>
>>     Here is a patch for this small problem with clang. It is not a blocking
>> issue for the 4.9 release but at the same time it is a rather safe fix so
>> just tell me if I can commit it.
>>
>>     All unordered_* tests run under Linux x86_64. I haven't clang installed
>> at the moment so a clang feedback would be appreciated.
>
> I'm going to test this with Clang to be sure it actually fixes the bug
> before we change anything, but it might have to wait for the weekend.
>
> N.B. please change the comment at the top of the patch to say "pool of
> nodes" instead of "pool of node"

This does fix the error with Clang.

Please go ahead and commit it, with the small change to the comment I
pointed out - thanks!
diff mbox

Patch

Index: include/bits/hashtable_policy.h
===================================================================
--- include/bits/hashtable_policy.h	(revision 206443)
+++ include/bits/hashtable_policy.h	(working copy)
@@ -161,7 +161,8 @@ 
       __hashtable_alloc& _M_h;
     };
 
-  // Functor similar to the previous one but without any pool of node to recycle.
+  // Functor similar to the previous one but without any pool of node to
+  // recycle.
   template<typename _NodeAlloc>
     struct _AllocNode
     {
@@ -1088,7 +1089,8 @@ 
 
       std::size_t
       _M_bucket_index(const __node_type* __p, std::size_t __n) const
-	noexcept( noexcept(declval<const _Hash&>()(declval<const _Key&>(), (std::size_t)0)) )
+	noexcept( noexcept(declval<const _Hash&>()(declval<const _Key&>(),
+						   (std::size_t)0)) )
       { return _M_ranged_hash()(_M_extract()(__p->_M_v()), __n); }
 
       void
@@ -1175,7 +1177,8 @@ 
       std::size_t
       _M_bucket_index(const __node_type* __p, std::size_t __n) const
 	noexcept( noexcept(declval<const _H1&>()(declval<const _Key&>()))
-		  && noexcept(declval<const _H2&>()((__hash_code)0, (std::size_t)0)) )
+		  && noexcept(declval<const _H2&>()((__hash_code)0,
+						    (std::size_t)0)) )
       { return _M_h2()(_M_h1()(_M_extract()(__p->_M_v())), __n); }
 
       void
Index: include/bits/hashtable.h
===================================================================
--- include/bits/hashtable.h	(revision 206443)
+++ include/bits/hashtable.h	(working copy)
@@ -260,9 +260,14 @@ 
 
       // Compile-time diagnostics.
 
+      // _Hash_code_base has everything protected, so use this derived type to
+      // access it.
+      struct __hash_code_base_access : __hash_code_base
+      { using __hash_code_base::_M_bucket_index; };
+
       // Getting a bucket index from a node shall not throw because it is used
       // in methods (erase, swap...) that shall not throw.
-      static_assert(noexcept(declval<const _Hashtable&>()
+      static_assert(noexcept(declval<const __hash_code_base_access&>()
 			     ._M_bucket_index((const __node_type*)nullptr,
 					      (std::size_t)0)),
 		    "Cache the hash code or qualify your functors involved"
@@ -277,15 +282,11 @@ 
 		    "Functor used to map hash code to bucket index"
 		    " must be default constructible");
 
-      // _Hash_code_base has a protected default constructor, so use this
-      // derived type to tell if it's usable.
-      struct __access_protected_ctor : __hash_code_base { };
-
       // 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<__access_protected_ctor>>::value,
+		    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");
 
Index: testsuite/23_containers/unordered_set/instantiation_neg.cc
===================================================================
--- testsuite/23_containers/unordered_set/instantiation_neg.cc	(revision 206443)
+++ testsuite/23_containers/unordered_set/instantiation_neg.cc	(working copy)
@@ -19,7 +19,7 @@ 
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// { dg-error "with noexcept" "" { target *-*-* } 265 }
+// { dg-error "with noexcept" "" { target *-*-* } 270 }
 
 #include <unordered_set>
 
Index: testsuite/23_containers/unordered_set/not_default_constructible_hash_neg.cc
===================================================================
--- testsuite/23_containers/unordered_set/not_default_constructible_hash_neg.cc	(revision 206443)
+++ testsuite/23_containers/unordered_set/not_default_constructible_hash_neg.cc	(working copy)
@@ -19,7 +19,7 @@ 
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// { dg-error "default constructible" "" { target *-*-* } 287 }
+// { dg-error "default constructible" "" { target *-*-* } 288 }
 
 #include <unordered_set>