Subject: Re: [boost] [histogram] review part 2 (implementation)
From: Hans Dembinski (hans.dembinski_at_[hidden])
Date: 2018-09-26 08:17:32
> On 26. Sep 2018, at 00:42, Steven Watanabe via Boost <boost_at_[hidden]> wrote:
> Destructors (along with constructors) are special.
> "The lifetime of an object of type T ends when ... the
> destructor call starts"
> "The properties ascribed to objects throughout this
> International Standard apply for a given object only
> during its lifetime"
> [basic.life] (C++11)
> And it gets worse. You're not just deallocating the
> buffer. You're also calling the destructors of all
> subobjects, including the label. The only thing that
> saves you from a double free is that you're using small
> labels that fit in the internal buffer in your tests.
Sorry, you are right. Additional proof:
Ok, no calling of destructors.
>> It is not overkill to use detail::buffer here instead of std::vector, since detail::buffer is anyway used by adaptive_storage. Replacing it here with std::vector does not make detail::buffer obsolete.
> My real complaint is not so much overkill as the
> difficulty of getting manual memory management right,
> which makes the code more error-prone and harder to
> understand. I believe that the problems I have
> pointed out are a sufficient demonstration of this.
Point taken and I learned my lesson.
>> static and dynamic histograms have the exact same behaviour in this regard.
> That may be what you intended, but it isn't
> what the code actually does. I can pass
> a one-element tuple to a dynamic histogram
> and it will be unpacked. Similarly, I can
> pass a (one element) vector to a (one axis)
> dynamic histogram and it will work. The
> latter has to be supported as the size cannot
> be detected at compile time.
Ok, that's a bug. I thought I unit-tested this case, but I didn't. Will be fixed.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk