You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Farid Zaripov <Fa...@epam.com> on 2008/01/22 14:20:30 UTC

[PATCH] remove explicit casting to void* when calling allocator<>::allocate(size, pointer)

  The some of the boost regression tests are using the following allocator:
 
namespace test
{
    template <class T>
    struct malloc_allocator
    {
[...]
        pointer allocate(size_type n, const_pointer u) { return allocate(n); }
[...]
    };
}
 
  The second parameter type is const_pointer, not malloc_allocator<void>::const_pointer.
As a result these tests are failed with the error:
------------------------------------------------------------------------------------------------------------------
include\rw/_tree.cc(140) : error C2664: '__rw::__rb_tree<_Key,_Val,_KeyOf,_Comp,_Alloc>::_C_node_buf *test::malloc_allocator<T>::allocate(test::malloc_allocator<T>::size_type,const __rw::__rb_tree<_Key,_Val,_KeyOf,_Comp,_Alloc>::_C_node_buf *)' : cannot convert parameter 2 from 'void *' to 'const __rw::__rb_tree<_Key,_Val,_KeyOf,_Comp,_Alloc>::_C_node_buf *'
------------------------------------------------------------------------------------------------------------------
 
 
  The patch below fixes these problems.
 
 
  ChangeLog:
  * include/rw/_tree.cc (_C_add_new_buffer): Remove casting the second
  parameter of the allocator(size, pointer) call to void*.
  * include/list.cc (_C_add_buffer): Ditto.
  * include/list (_C_get_node): Ditto.
 
------------------------------------------------------------------------------------------------------------------
Index: include/rw/_tree.cc
===================================================================
--- include/rw/_tree.cc (revision 614178)
+++ include/rw/_tree.cc (working copy)
@@ -137,11 +137,11 @@
         __newsize = __bufsize + 1;
 
     _C_buf_ptr_t __buf = 
-        _C_buf_alloc_t (*this).allocate (size_type (1), (void*)_C_buf_list);
+        _C_buf_alloc_t (*this).allocate (size_type (1), _C_buf_list);
 
     _TRY {
         __buf->_C_buffer = 
-            _C_node_alloc_t (*this).allocate (__newsize, (void*)_C_last);
+            _C_node_alloc_t (*this).allocate (__newsize, _C_last);
     }
     _CATCH (...) {
         _C_buf_alloc_t (*this).deallocate (__buf, 1);
Index: include/list.cc
===================================================================
--- include/list.cc (revision 614178)
+++ include/list.cc (working copy)
@@ -86,12 +86,12 @@
         }          
      }
     _C_buf_pointer __tmp = 
-        _C_buf_alloc_type (*this).allocate (1, (void*)_C_buflist);
+        _C_buf_alloc_type (*this).allocate (1, _C_buflist);
 
     _TRY {
         __tmp->_C_buffer = 
            _C_node_alloc_type (*this).allocate (__next_buffer_size,
-                                                (void*)_C_last);
+                                                _C_last);
     }
     _CATCH (...) {
         _C_buf_alloc_type (*this).deallocate (__tmp, 1);
Index: include/list
===================================================================
--- include/list (revision 614178)
+++ include/list (working copy)
@@ -335,7 +335,7 @@
         insert (b,e,v)
 
     _C_link_type _C_get_node (bool = false) {
-        return _C_node_alloc_type (*this).allocate (1, (void*)_C_last);
+        return _C_node_alloc_type (*this).allocate (1, _C_last);
     }
 
     void _C_put_node (_C_link_type __link) {
------------------------------------------------------------------------------------------------------------------
 
Farid.
 

Re: [PATCH] remove explicit casting to void* when calling allocator<>::allocate(size, pointer)

Posted by Martin Sebor <se...@roguewave.com>.
Farid Zaripov wrote:
> From: Martin Sebor от имени Martin Sebor
> Sent: Tue, 22.01.2008 18:20
> To: dev@stdcxx.apache.org
> Subject: Re: [PATCH] remove explicit casting to void* when calling allocator<>::allocate(size, pointer)
> 
>>>   The second parameter type is const_pointer, not malloc_allocator<void>::const_pointer.
>>> As a result these tests are failed with the error:
> 
>> Are you sure that the allocator is correct?
>  
>   No. And I'll make the corresponding ticket in boost trac, but it's easier to fix problem at
> our side (if it's possible) than wait until somebody from boost team fixes the problem in boost.
> Another reason is succeeding of the such tests compiled with other STL's.

As long as we don't compromise the conformance or quality of our
implementation in the process.

> 
> [...]
> 
>> IIRC, the cast to void* is there to make user-defined pointers work
>> but I don't think it's correct either. I can't think of any way to
>> solve this than to make allocate a template for allocators with
>> user-defined pointers:
>  
>   But why in the list and tree classes explicit cast is used? Implicit cast to const void* should work.
> Hint version of allocate() is also used in deque and vector, but without explicit cast to void*.

I'm not sure why. I don't like the casts so if they're not necessary
I'm all for removing them. To decide, I suspect we'll need to exercise
each of the containers with an allocator that encapsulates user-defined
pointer types.

I have this hazy recollection that I added at least one such test not
so long ago, and that the test had originally been a part of a bigger
test exercising all containers with user-defined pointers. The test
was failing because our list doesn't allow such things. I must be
hallucinating though because I can't find either the issue or the
test...

Martin

> 
>> There's a test (20_allocator, I think) that we haven't migrated to
>> svn yet that exercises containers with allocators with user-defined
>> pointers. You might want to make sure it compiles with the changes
>> you're proposing.
>  
>   Of course.
> 
> Farid.
>  


Re: [PATCH] remove explicit casting to void* when calling allocator<>::allocate(size, pointer)

Posted by Farid Zaripov <Fa...@epam.com>.
From: Martin Sebor от имени Martin Sebor
Sent: Tue, 22.01.2008 18:20
To: dev@stdcxx.apache.org
Subject: Re: [PATCH] remove explicit casting to void* when calling allocator<>::allocate(size, pointer)

>>   The second parameter type is const_pointer, not malloc_allocator<void>::const_pointer.
>> As a result these tests are failed with the error:

> Are you sure that the allocator is correct?
 
  No. And I'll make the corresponding ticket in boost trac, but it's easier to fix problem at
our side (if it's possible) than wait until somebody from boost team fixes the problem in boost.
Another reason is succeeding of the such tests compiled with other STL's.

[...]

> IIRC, the cast to void* is there to make user-defined pointers work
> but I don't think it's correct either. I can't think of any way to
> solve this than to make allocate a template for allocators with
> user-defined pointers:
 
  But why in the list and tree classes explicit cast is used? Implicit cast to const void* should work.
Hint version of allocate() is also used in deque and vector, but without explicit cast to void*.

> There's a test (20_allocator, I think) that we haven't migrated to
> svn yet that exercises containers with allocators with user-defined
> pointers. You might want to make sure it compiles with the changes
> you're proposing.
 
  Of course.

Farid.
 

Re: [PATCH] remove explicit casting to void* when calling allocator<>::allocate(size, pointer)

Posted by Martin Sebor <se...@roguewave.com>.
Farid Zaripov wrote:
>   The some of the boost regression tests are using the following allocator:
>  
> namespace test
> {
>     template <class T>
>     struct malloc_allocator
>     {
> [...]
>         pointer allocate(size_type n, const_pointer u) { return allocate(n); }
> [...]
>     };
> }
>  
>   The second parameter type is const_pointer, not malloc_allocator<void>::const_pointer.
> As a result these tests are failed with the error:

Are you sure that the allocator is correct?

The Allocator Requirements table says that in calls to allocate(n, u),
u is a value of type Y::const_pointer, and Y is supposed to be the
corresponding allocator for type U. Assuming const_pointer depends
on T, I'm not sure I see how malloc_allocator could guarantee that
such conversions are valid between any two specializations of the
class.

IIRC, the cast to void* is there to make user-defined pointers work
but I don't think it's correct either. I can't think of any way to
solve this than to make allocate a template for allocators with
user-defined pointers:

   template <class U>
   pointer allocate(size_type, typename allocator<U>::const_pointer);

Unfortunately, this would make the template argument non-deducible,
so maybe just:

   template <class U>
   pointer allocate(size_type, U) {
       // ASSUME (U == Allocator<V>::const_pointer);
   }

There's a test (20_allocator, I think) that we haven't migrated to
svn yet that exercises containers with allocators with user-defined
pointers. You might want to make sure it compiles with the changes
you're proposing.

Martin

> ------------------------------------------------------------------------------------------------------------------
> include\rw/_tree.cc(140) : error C2664: '__rw::__rb_tree<_Key,_Val,_KeyOf,_Comp,_Alloc>::_C_node_buf *test::malloc_allocator<T>::allocate(test::malloc_allocator<T>::size_type,const __rw::__rb_tree<_Key,_Val,_KeyOf,_Comp,_Alloc>::_C_node_buf *)' : cannot convert parameter 2 from 'void *' to 'const __rw::__rb_tree<_Key,_Val,_KeyOf,_Comp,_Alloc>::_C_node_buf *'
> ------------------------------------------------------------------------------------------------------------------
>  
>  
>   The patch below fixes these problems.
>  
>  
>   ChangeLog:
>   * include/rw/_tree.cc (_C_add_new_buffer): Remove casting the second
>   parameter of the allocator(size, pointer) call to void*.
>   * include/list.cc (_C_add_buffer): Ditto.
>   * include/list (_C_get_node): Ditto.
>  
> ------------------------------------------------------------------------------------------------------------------
> Index: include/rw/_tree.cc
> ===================================================================
> --- include/rw/_tree.cc (revision 614178)
> +++ include/rw/_tree.cc (working copy)
> @@ -137,11 +137,11 @@
>          __newsize = __bufsize + 1;
>  
>      _C_buf_ptr_t __buf = 
> -        _C_buf_alloc_t (*this).allocate (size_type (1), (void*)_C_buf_list);
> +        _C_buf_alloc_t (*this).allocate (size_type (1), _C_buf_list);
>  
>      _TRY {
>          __buf->_C_buffer = 
> -            _C_node_alloc_t (*this).allocate (__newsize, (void*)_C_last);
> +            _C_node_alloc_t (*this).allocate (__newsize, _C_last);
>      }
>      _CATCH (...) {
>          _C_buf_alloc_t (*this).deallocate (__buf, 1);
> Index: include/list.cc
> ===================================================================
> --- include/list.cc (revision 614178)
> +++ include/list.cc (working copy)
> @@ -86,12 +86,12 @@
>          }          
>       }
>      _C_buf_pointer __tmp = 
> -        _C_buf_alloc_type (*this).allocate (1, (void*)_C_buflist);
> +        _C_buf_alloc_type (*this).allocate (1, _C_buflist);
>  
>      _TRY {
>          __tmp->_C_buffer = 
>             _C_node_alloc_type (*this).allocate (__next_buffer_size,
> -                                                (void*)_C_last);
> +                                                _C_last);
>      }
>      _CATCH (...) {
>          _C_buf_alloc_type (*this).deallocate (__tmp, 1);
> Index: include/list
> ===================================================================
> --- include/list (revision 614178)
> +++ include/list (working copy)
> @@ -335,7 +335,7 @@
>          insert (b,e,v)
>  
>      _C_link_type _C_get_node (bool = false) {
> -        return _C_node_alloc_type (*this).allocate (1, (void*)_C_last);
> +        return _C_node_alloc_type (*this).allocate (1, _C_last);
>      }
>  
>      void _C_put_node (_C_link_type __link) {
> ------------------------------------------------------------------------------------------------------------------
>  
> Farid.
>