Re: [boost] Review Request: impl_ptr (pimpl)

Subject: Re: [boost] Review Request: impl_ptr (pimpl)
From: Vladimir Batov (Vladimir.Batov_at_[hidden])
Date: 2017-08-22 22:02:19

On 08/23/2017 07:41 AM, Gary Furnish via Boost wrote:
> On Tue, Aug 22, 2017 at 3:18 PM, Vladimir Batov via Boost
> <boost_at_[hidden]> wrote:
>> On 08/23/2017 01:06 AM, Gary Furnish via Boost wrote:
>>> While I like the idea in general (bikeshedding aside), it should not
>>> be possible to end up with nulls when treating something as a value
>>> type.
>> Is it a general observation or you have a particular use-case in mind that
>> involves the discussed impl_ptr? It that is the former, then IMO it is not
>> an issue as impl_ptr is a base class and the user/interface/proxy class
>> ultimately decides if it is "possible to end up with nulls" or not. If that
>> is the latter, then I'd certainly like to look at it and to evaluate and to
>> fix if necessary.
> On the nulls: just looking it may be policy dependent if you can end
> up with nulls in the sense of runtime, but the machinery for nulls
> will still show up in auto completion,name resolution, etc unless it
> is enable-ifed or specialized out. It is possible I mistook how this
> works though. I don't see a problem with supporting shared situations
> with policies, but it should be 100% disabled.  No
>
> static other_type null() { return boost_impl_ptr_detail<other_type>::null(); }
> static user_type null() { return boost_impl_ptr_detail< user_type>::null(); }

Sure. If taking these convenience interfaces out makes people happy, I 
have nothing against it. No drama. I guess, I'll go take it out right 
away. :-)

>
>>> All of that should be hidden away with some sort of enable if
>>> machinery.  Also your support for allocators doesn't work properly
>>> because if you use something like an allocator that returns offset
>>> pointers you assume that all allocators return normal pointers.
>> Well, "your support for allocators doesn't work properly" seems like a
>> sweeping statement. They "seem" to work for me but you must have a
>> particular use-case in mind where they do not. Do you mind elaborating,
>> sharing it? I am somewhat puzzled by the meaning of "offset pointers". I
>> presume it is a pointer offset from the beginning of actually allocated
>> block, right? If so, does std::allocator support that?.. impl_ptr uses the
>> facilities provided by the allocator concept. Namely, if your particular
>> allocator returns an "offset pointer" from allocate(), then that same
>> pointer is given back to your allocator to deal with inside deallocate().
>> So, where does "your support for allocators doesn't work properly" come in?
>>
> Offset pointers are pointers that you use with an allocator where the
> pointer is actually a size_t that is added to the location of the
> pointer.  This is used in mmap situations where you do not know the
> absolute address, only relative to the current pointer offset.  See
> http://www.boost.org/doc/libs/1_65_0/doc/html/interprocess/offset_ptr.html
>   So you just define an allocator where Allocator::pointer =
> boost::offset_ptr<T> and vector etc automatically work in shared
> memory.  The basic punchline is instead of hardcoding the allocator
> return value as T* you want to capture a
> std::allocator_traits<...>::pointer .  You can then use
> std::address_of(*raw memory) to get the raw memory address.  (Note
> address of because operator& could be overloaded).
> (This is how to do it in c++17, earlier versions require a bit more
> boiler plate).  It is not true allocator support if you don't support
> the rarer cases.

Uh, right. Thank you for the link and explanations. Appreciated. And you 
are certainly correct, I should be using 
std::allocator_traits<...>::pointer. That said, I consider it an 
oversight, a nit to be addressed in due course... Agree?