Boost logo

Boost :

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"
> [] (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, gregod at, cpdaniel at, john at